Repository navigation
fix(dsql): correct partial index, expression index, and function support - #278
Merged
Merged
Conversation
praba2210
force-pushed
the
fix/dsql-index-and-function-support
branch
from
September 30, 2026 18:25
612f8a1 to
992df3d
Compare
praba2210
marked this pull request as ready for review
September 30, 2026 18:33
praba2210
requested review from
a team,
arnewouters,
krokoko and
laithalsaadoon
September 30, 2026 18:33
praba2210
requested review from
Benjscho,
amaksimo,
anwesham-lab,
gxjx-x,
jaichabria,
pkale and
spencercorwin
September 30, 2026 18:33
spencercorwin
left a comment
spencercorwin
left a comment
There was a problem hiding this comment.
One additional release item: this changes shipped databases-on-aws skill behavior, but the plugin versions remain at 1.9.0. Per the repository's maintainer guide, bug fixes require a patch bump. Please update .claude-plugin/marketplace.json, plugins/databases-on-aws/.claude-plugin/plugin.json, and plugins/databases-on-aws/.codex-plugin/plugin.json to 1.9.1.
praba2210
force-pushed
the
fix/dsql-index-and-function-support
branch
from
October 1, 2026 16:40
992df3d to
02673b3
Compare
spencercorwin
left a comment
spencercorwin
left a comment
There was a problem hiding this comment.
One source-link issue that cannot be attached inline because the line is unchanged: the Asynchronous Indexes link at pg-migrations/index-conversion.md:19 points to working-with-indexes.html, which currently redirects to the DSQL introduction page. Please replace it with the published CREATE INDEX syntax documentation and/or Asynchronous indexes in Aurora DSQL. The CREATE INDEX page is the direct source for ASYNC, expression indexes, partial indexes, immutability, and predicate implication.
praba2210
force-pushed
the
fix/dsql-index-and-function-support
branch
from
October 1, 2026 19:11
02673b3 to
555f6de
Compare
praba2210
pushed a commit
that referenced
this pull request
Oct 1, 2026
troubleshooting.md covered only 40001 and 23503, and five of its quoted error messages did not match what DSQL emits. One shared no words at all with the real text, so an agent matching on it matches nothing. Adds a Limits and Error Codes section: 21 limits and 20 rejections, each with its SQLSTATE and exact message, plus the documented cluster quotas marked unverified. 54000 and 0A000 each span more than a dozen rows, so the message is the discriminator, not the code. Three corrections change the advice: - OC001 is 40001, the same code SKILL.md routes into the OCC retry loop. OC000 is the DML conflict and clears under backoff; OC001 is concurrent DDL and does not. occ-retry-patterns.md now says so. - "unable to accept connection" is a prefix shared by five conditions. Only the Signature expired hint means the token is stale; for the rest, regenerating it will not help. - An oversized protocol message closes the connection with no error, so the commit outcome is unknown and a bare retry can double-write. Corrects the key-size limit to 2 KiB encoded, about 1,981 bytes usable, shared across all key columns and counted in bytes, and the 24-index limit to include the primary key, so a table with one accepts 23 secondary indexes. The eval grader accepted "24 secondary indexes" and a fixture pinned it as passing, so the suite graded the error this commit fixes as a correct answer. Also drops ASC/DESC from two index examples that return 0A000 today. A direction-free index serves a uniformly ordered ORDER BY by backward scan, and only mixed ordering adds a sort. One DESC remains in the Partial Index Conversion block that #278 deletes. Bumps the plugin version to 1.9.1.
praba2210
force-pushed
the
fix/dsql-index-and-function-support
branch
from
October 1, 2026 19:51
555f6de to
9db44c8
Compare
Contributor
Author
|
All review concerns from both rounds are addressed in Round 1
Round 2
|
praba2210
pushed a commit
that referenced
this pull request
Oct 1, 2026
troubleshooting.md covered only 40001 and 23503, and five of its quoted error messages did not match what DSQL emits. One of them matches nothing in the real text at all, so an agent looking it up finds nothing. Adds a Limits and Error Codes section: 21 limits and 20 rejections, each with its SQLSTATE and the exact message where one exists, plus the documented cluster quotas. 54000 covers 15 of those rows and 0A000 covers 13, so within the section the message is the discriminator, not the code. Four corrections change the advice: - OC001 is 40001, the same code SKILL.md routes into the OCC retry loop. OC000 is the DML conflict and clears under backoff; OC001 is concurrent DDL and keeps recurring while another session issues it. Both stay in the loop, and the sample now logs which one it saw. - "unable to accept connection" is a prefix shared by five conditions. Only the Signature expired hint means the token is stale, and even then regenerating in a loop cannot fix a skewed host clock. - An EOF at or after COMMIT leaves the commit outcome unknown whatever the cause, so a bare retry can double-write. - index-conversion.md said dsql_lint preserves index sort order. It strips it, and reports that as a warning, so a pipeline gating on errors == 0 never sees the statement change. Corrects the key-size limit to about 1,981 usable bytes, shared across all key columns and counted in bytes, and the 24-index limit to include the primary key, so a table with one accepts 23 secondary indexes. Key size, temp space and view-definition size all differ from the quotas page; each row now says so rather than leaving a reader to find the contradiction. The eval grader accepted "24 secondary indexes" and a fixture pinned it as passing, so the suite graded the error this commit fixes as a correct answer. The grader now rejects that framing worded either way, additively or subtractively, while still accepting an answer that quotes the wrong form in order to correct it. Nineteen cases pin the phrasings and two pin the failure evidence. Also drops DESC from two index examples that return 0A000 today. A direction-free index serves a uniformly ordered ORDER BY by backward scan, and only mixed ordering adds a sort. One DESC remains in the Partial Index Conversion block that #278 deletes. Bumps the plugin version to 1.10.0.
praba2210
pushed a commit
that referenced
this pull request
Oct 1, 2026
troubleshooting.md covered only 40001 and 23503, and five of its quoted error messages did not match what DSQL emits. One of them matches nothing in the real text at all, so an agent looking it up finds nothing. Adds a Limits and Error Codes section: 21 limits and 20 rejections, each with its SQLSTATE and the exact message where one exists, plus the documented cluster quotas. 54000 covers 15 of those rows and 0A000 covers 13, so within the section the message is the discriminator, not the code. Four corrections change the advice: - OC001 is 40001, the same code SKILL.md routes into the OCC retry loop. OC000 is the DML conflict and clears under backoff; OC001 is concurrent DDL and keeps recurring while another session issues it. Both stay in the loop, and the sample now logs which one it saw. - "unable to accept connection" is a prefix shared by five conditions. Only the Signature expired hint means the token is stale, and even then regenerating in a loop cannot fix a skewed host clock. - An EOF at or after COMMIT leaves the commit outcome unknown whatever the cause, so a bare retry can double-write. - index-conversion.md said dsql_lint preserves index sort order. It strips it, and reports that as a warning, so a pipeline gating on errors == 0 never sees the statement change. Corrects the key-size limit to about 1,981 usable bytes, shared across all key columns and counted in bytes, and the 24-index limit to include the primary key, so a table with one accepts 23 secondary indexes. Key size, temp space and view-definition size all differ from the quotas page; each row now says so rather than leaving a reader to find the contradiction. The eval grader accepted "24 secondary indexes" and a fixture pinned it as passing, so the suite graded the error this commit fixes as a correct answer. The grader now rejects that framing worded either way, additively or subtractively, while still accepting an answer that quotes the wrong form in order to correct it. Nineteen cases pin the phrasings and two pin the failure evidence. Also drops DESC from two index examples that return 0A000 today. A direction-free index serves a uniformly ordered ORDER BY by backward scan, and only mixed ordering adds a sort. One DESC remains in the Partial Index Conversion block that #278 deletes. Bumps the plugin version to 1.10.0.
praba2210
pushed a commit
that referenced
this pull request
Oct 1, 2026
troubleshooting.md covered only 40001 and 23503, and five of its quoted error messages did not match what DSQL emits. One of them matches nothing in the real text at all, so an agent looking it up finds nothing. Adds a Limits and Error Codes section: 21 limits and 20 rejections, each with its SQLSTATE and the exact message where one exists, plus the documented cluster quotas. 54000 covers 15 of those rows and 0A000 covers 13, so within the section the message is the discriminator, not the code. Four corrections change the advice: - OC001 is 40001, the same code SKILL.md routes into the OCC retry loop. OC000 is the DML conflict and clears under backoff; OC001 is concurrent DDL and keeps recurring while another session issues it. Both stay in the loop, and the sample now logs which one it saw. - "unable to accept connection" is a prefix shared by five conditions. Only the Signature expired hint means the token is stale, and even then regenerating in a loop cannot fix a skewed host clock. - An EOF at or after COMMIT leaves the commit outcome unknown whatever the cause, so a bare retry can double-write. - index-conversion.md said dsql_lint preserves index sort order. It strips it, and reports that as a warning, so a pipeline gating on errors == 0 never sees the statement change. Corrects the key-size limit to about 1,981 usable bytes, shared across all key columns and counted in bytes, and the 24-index limit to include the primary key, so a table with one accepts 23 secondary indexes. Key size, temp space and view-definition size all differ from the quotas page; each row now says so rather than leaving a reader to find the contradiction. The eval grader accepted "24 secondary indexes" and a fixture pinned it as passing, so the suite graded the error this commit fixes as a correct answer. The grader now rejects that framing worded either way, additively or subtractively, while still accepting an answer that quotes the wrong form in order to correct it. Nineteen cases pin the phrasings and two pin the failure evidence. Also drops DESC from two index examples that return 0A000 today. A direction-free index serves a uniformly ordered ORDER BY by backward scan, and only mixed ordering adds a sort. One DESC remains in the Partial Index Conversion block that #278 deletes. Bumps the plugin version to 1.10.0.
spencercorwin
requested changes
Oct 1, 2026
DSQL supports partial indexes, expression indexes, and SQL-language functions. The skill said otherwise and prescribed conversions that are not needed: - partial indexes: told users to drop the WHERE clause - expression indexes: told users to add a GENERATED column and rewrite their queries - CREATE FUNCTION: routed all functions to the application layer All three are documented as supported and were verified against a cluster. dsql_lint reports no diagnostics for any of them, and has no index_partial or index_expression rule. The expression-index section also contradicted dsql-lint.md: it used ALTER TABLE ADD COLUMN ... GENERATED ALWAYS AS ... STORED, which fails with "ADD COLUMN with constraint not supported". Replaced with the fix that works — making the expression IMMUTABLE by pinning the time zone. The JSONB GIN section prescribed the same failing ALTER TABLE, so it now indexes the extraction expression directly and eval 203 grades that answer instead. Bounds the GENERATED claim in schema-objects.md to CREATE TABLE, where it does work. Operator classes needed a warning: dsql_lint strips USING but leaves the opclass in place and reports no error, so the SQL it hands back cannot be created. Same class of trap for BEGIN ATOMIC function bodies, where the returned fixed_sql is split at the semicolon inside the body. Drops the partial and expression branches from the flowchart. A partial index is a btree index, so the first branch already covers both; the btree terminal now names WHERE and IMMUTABLE expressions. Also swaps the two non-existent lint rules for index_volatile_function, which is real but was undocumented, adds a CREATE FUNCTION section to pg-migrations/schema-objects.md, updates evals 203, 207 and 208, which graded the old behavior as correct, and bumps the plugin to 1.9.1. Routing now reaches the new material: the index-conversion.md row in SKILL.md no longer requires a lint diagnostic, the schema-objects.md row names CREATE FUNCTION, and Workflow 10 loads index-conversion.md. Smaller corrections found while checking the above against a cluster: geometric types are rejected outright rather than stored as text; a partial index is only eligible for a query that implies its predicate, not guaranteed to be used; LANGUAGE sql may be omitted when the body is RETURN or BEGIN ATOMIC, so neither the conversion checklist nor the MySQL table should test for that clause; and parse_error needs an exception in dsql-lint.md, since no re-lint of a BEGIN ATOMIC body will ever come back clean. Repoints the async index link, which redirected to the user guide root.
praba2210
force-pushed
the
fix/dsql-index-and-function-support
branch
from
October 2, 2026 19:19
9db44c8 to
10d8095
Compare
spencercorwin
approved these changes
Oct 2, 2026
spencercorwin
left a comment
spencercorwin
left a comment
There was a problem hiding this comment.
Reviewed the updated head. The index_volatile_function resolution now preserves semantics by preferring an equivalent immutable expression, and all checks pass. The two-PR sequence is appropriate: this PR establishes the corrected partial-index, expression-index, and SQL-function guidance; #279 can then rebase onto it and retain these changes while handling the pre-existing index sort-order guidance and its separate limits/error-reference scope.
krokoko
approved these changes
Oct 2, 2026
praba2210
pushed a commit
that referenced
this pull request
Oct 5, 2026
troubleshooting.md covered only 40001 and 23503, and five of its quoted error messages did not match what DSQL emits. One of them matches nothing in the real text at all, so an agent looking it up finds nothing. Adds a Limits and Error Codes section: 21 limits and 20 rejections, each with its SQLSTATE and the exact message where one exists, plus the documented cluster quotas. 54000 covers 15 of those rows and 0A000 covers 13, so within the section the message is the discriminator, not the code. Four corrections change the advice: - OC001 is 40001, the same code SKILL.md routes into the OCC retry loop. OC000 is the DML conflict and clears under backoff; OC001 is concurrent DDL and keeps recurring while another session issues it. Both stay in the loop, and the sample now logs which one it saw. - "unable to accept connection" is a prefix shared by five conditions. Only the Signature expired hint means the token is stale, and even then regenerating in a loop cannot fix a skewed host clock. - An EOF at or after COMMIT leaves the commit outcome unknown whatever the cause, so a bare retry can double-write. - index-conversion.md said dsql_lint preserves index sort order. It strips it, and reports that as a warning, so a pipeline gating on errors == 0 never sees the statement change. Corrects the key-size limit to about 1,981 usable bytes, shared across all key columns and counted in bytes, and the 24-index limit to include the primary key, so a table with one accepts 23 secondary indexes. Key size, temp space and view-definition size all differ from the quotas page; each row now says so rather than leaving a reader to find the contradiction. The eval grader accepted "24 secondary indexes" and a fixture pinned it as passing, so the suite graded the error this commit fixes as a correct answer. The grader now rejects that framing worded either way, additively or subtractively, while still accepting an answer that quotes the wrong form in order to correct it. Nineteen cases pin the phrasings and two pin the failure evidence. Also drops DESC from two index examples that return 0A000 today. A direction-free index serves a uniformly ordered ORDER BY by backward scan, and only mixed ordering adds a sort. One DESC remains in the Partial Index Conversion block that #278 deletes. Bumps the plugin version to 1.10.0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partial indexes, expression indexes, and SQL-language functions are all supported in Aurora DSQL. The skill says they are not, and routes users to workarounds they do not need — one of which,
ALTER TABLE ... ADD COLUMN ... GENERATED, DSQL rejects outright.Related
#279 follows this PR with a limits and error-code reference for
troubleshooting.md. Two notes for reviewer:sort orderfrommain— DSQL rejects it with0A000: specifying sort order not supported for index keys. docs(dsql): add limits/error-code reference, fix wrong messages #279 drops it; theWHERE, IMMUTABLE expressionsadded here is unaffected.index-conversion.mdrouting row inSKILL.mdand the three manifests, sothey conflict. This one is further along, so it should land first and docs(dsql): add limits/error-code reference, fix wrong messages #279 will rebase.
Changes
Corrections
WHEREpg-migrations/index-conversion.mdGENERATEDcolumnpg-migrations/index-conversion.mdALTER TABLE ADD COLUMN ... GENERATED ALWAYS AS ... STOREDAT TIME ZONEto make the expression IMMUTABLE — theALTERfails with0A000, contradictingdsql-lint.md:113pg-migrations/index-conversion.mdCREATE PROCEDURE/CREATE FUNCTION→ app layerCREATE FUNCTIONconverts toLANGUAGE sql; onlyCREATE PROCEDUREneeds the app layermysql-migrations/type-mapping.mdindex_partial,index_expressiondsql-lint.mdGIN/GiST/BRIN → btree, operator class removalSKILL.mdAdditions
## CREATE FUNCTION → LANGUAGE sql Only— three body forms, plus rejectedplpgsqlandCREATE PROCEDURE.pg-migrations/had no function guidancepg-migrations/schema-objects.mdindex_volatile_functionto the unfixable-rules table — real but undocumenteddsql-lint.mdexpression-index-preserved, second non-immutable index added so one case covers both sides. Its first expectation was "States that expression indexes are not supported" — without the change the corrected skill fails its own eval. Eval 207 now expects the predicate keptpg_migration_evals.jsonVerification
CREATE INDEXdocuments[ WHERE predicate ],CREATE INDEX ASYNC ON films ((lower(title)));, and predicate implication.CREATE FUNCTION: "where LANGUAGE must be sql".dsql_lint: 0 diagnostics on every statement added here.INCLUDE+WHEREindexes reachindisvalid = tand are used by the planner.LANGUAGE sqlfunctions execute;plpgsqlandCREATE PROCEDUREreturn0A000.extract(year FROM created_at)ontimestamptzreturns42P17as both an index and a generation expression;AT TIME ZONE 'UTC'fixes it, a bare::timestampcast does not.mise run build: passes. Functional evals not run — they need--mcp-config.SKILL.mdis a one-cell change;dprintreflowed the table, hence the diff size.Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.