Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: EXPOSED-728 [SQLite] Remove ENABLE_UPDATE_DELETE_LIMIT metadata check from core function provider #2402

Merged
merged 3 commits into from
Feb 12, 2025

Conversation

bog-walk
Copy link
Member

Description

Summary of the change: Deprecate SQLiteDialect.ENABLE_UPDATE_DELETE_LIMIT and replace with metadata functions that perform the same query, but which can be extracted into separate modules when R2DBC is introduced.

Detailed description:

  • Why:

SQLiteDialect has a companion property that sends a low-level JDBC connection query to check if LIMIT clause is allowed in either update or delete statements. The latter is only possible if SQLite is built from the source with the compile-time flag ENABLE_UPDATE_DELETE_LIMIT enabled (source docs).

In preparation for R2DBC, this query needs to be removed from exposed-core so that it can be implemented in both a blocking and non-blocking way.
FunctionProvider is not expected to have jdbc vs r2dbc versions. So the metadata check is instead placed in ExposedDatabaseMetadata and invoked in update() and delete() directly, since they will all be extracted to their respective modules.

This was missed from previous refactors because it doesn't use exec().

  • How:
    • Deprecate SQLiteDialect.ENABLE_UPDATE_DELETE_LIMIT
    • Replace with DatabaseDialect.supportsLimitWithUpdateOrDelete(), which calls ExposedDatabaseMetadata.supportsLimitWithUpdateOrDelete() to send the same query with SQLite. All other dialects are hard-coded with a boolean value.
    • Remove this check from SQLiteDialect's function provider and place directly in Table.update() and Table.delete variants.

Type of Change

Please mark the relevant options with an "X":

  • Other - refactor
  • Deprecation

Affected databases:

  • SQLite

Checklist

  • Unit tests are in place
  • The build is green (including the Detekt check)
  • All public methods affected by my PR has up to date API docs

Related Issues

EXPOSED-728

@bog-walk bog-walk requested a review from e5l February 11, 2025 03:41
…data

check from core function provider

SqliteDialect has a companion property that sends a low-level JDBC connection
query to check if LIMIT clause is allowed in either update or delete statements.
In preparation for R2DBC, this metadata query is removed from the core module and
replaced with appropriate DatabaseDialect and ExposedDatabaseMetadata functions that
can be called in appropriate blocking/non-blocking ways.

Tests have been adjusted accordingly.
…data check from core function provider

Fix strange import omptimization changes.
…data check from core function provider

Fix incorrect test exclusion
@bog-walk bog-walk force-pushed the bog-walk/refactor-sqlite-limit-check branch from 50c20e5 to 446040d Compare February 12, 2025 01:33
@bog-walk bog-walk merged commit 8df77cb into main Feb 12, 2025
7 checks passed
@bog-walk bog-walk deleted the bog-walk/refactor-sqlite-limit-check branch February 12, 2025 02:16
joc-a pushed a commit that referenced this pull request Feb 13, 2025
…data check from core function provider (#2402)

* refactor: EXPOSED-728 [SQLite] Remove ENABLE_UPDATE_DELETE_LIMIT metadata
check from core function provider

SqliteDialect has a companion property that sends a low-level JDBC connection
query to check if LIMIT clause is allowed in either update or delete statements.
In preparation for R2DBC, this metadata query is removed from the core module and
replaced with appropriate DatabaseDialect and ExposedDatabaseMetadata functions that
can be called in appropriate blocking/non-blocking ways.

Tests have been adjusted accordingly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants