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

Insights proposed index wrong SQL statement #119579

Closed
gerwim opened this issue Feb 23, 2024 · 2 comments · Fixed by #121731
Closed

Insights proposed index wrong SQL statement #119579

gerwim opened this issue Feb 23, 2024 · 2 comments · Fixed by #121731
Assignees
Labels
A-observability-inf C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community T-observability X-blathers-triaged blathers was able to find an owner

Comments

@gerwim
Copy link

gerwim commented Feb 23, 2024

Describe the problem

Creating an index fails:
Screenshot 2024-02-23 at 16 51 45

The error (from network tab):

{"error":{"code":"42601","message":"parsing statement 1: at or near \"_\": syntax error","detail":"source SQL:\nCREATE INDEX IF NOT EXISTS \"offers\"_\"startdate\"_storing_rec_idx ON whatsuper_tst.\"Offers\".\"Offers\" (\"StartDate\") STORING (\"Name\", \"SubcategoryId\", \"SupermarketId\", \"PriceShort\", \"PriceOld\", \"PriceLabel\", \"PriceUnit\", \"ExtraText\", \"ImageUrl\", \"ImageUrlOriginal\", \"EndDate\", \"MetadataId\", \"NeedsReview\", \"Hidden\", \"ScraperRunId\")\n                                   ^\n--\nCREATE INDEX IF NOT EXISTS \"offers\"_\"startdate\"_storing_rec_idx ON whatsuper_tst.\"Offers\".\"Offers\" (\"StartDate\") STORING (\"Name\", \"SubcategoryId\", \"SupermarketId\", \"PriceShort\", \"PriceOld\", \"PriceLabel\", \"PriceUnit\", \"ExtraText\", \"ImageUrl\", \"ImageUrlOriginal\", \"EndDate\", \"MetadataId\", \"NeedsReview\", \"Hidden\", \"ScraperRunId\")","hint":"try \\h CREATE INDEX","severity":"ERROR","source":{"file":"lexer.go","line":404,"function":"Error"}}}

To Reproduce

This happens when applying a suggested index.

If possible, provide steps to reproduce the behavior:

  1. Go to insights
  2. Go to schema insights
  3. filter on insight typoe "create index"
  4. Click "create index"

Expected behavior
The index is created

Additional data / screenshots
Replace index is broken too, but I'm not sure if it's related (didn't look into it, but I assume it is).

Generated SQL:

CREATE INDEX IF NOT EXISTS "offers"_"startdate"_storing_rec_idx ON whatsuper_tst."Offers"."Offers" ("StartDate") STORING ("Name", "SubcategoryId", "SupermarketId", "PriceShort", "PriceOld", "PriceLabel", "PriceUnit", "ExtraText", "ImageUrl", "ImageUrlOriginal", "EndDate", "MetadataId", "NeedsReview", "Hidden", "ScraperRunId"); 

The issue is caused by the double quotes around offers and startdate. This is a valid SQL statement:

CREATE INDEX IF NOT EXISTS offers_startdate_storing_rec_idx ON whatsuper_tst."Offers"."Offers" ("StartDate") STORING ("Name", "SubcategoryId", "SupermarketId", "PriceShort", "PriceOld", "PriceLabel", "PriceUnit", "ExtraText", "ImageUrl", "ImageUrlOriginal", "EndDate", "MetadataId", "NeedsReview", "Hidden", "ScraperRunId");

Environment:

  • CockroachDB version: v23.2.1
  • Server OS: Debian 12, running the docker container cockroachdb/cockroach:v23.2.1
  • Client app: dashboard

Jira issue: CRDB-36229

@gerwim gerwim added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Feb 23, 2024
Copy link

blathers-crl bot commented Feb 23, 2024

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I have CC'd a few people who may be able to assist you:

  • @cockroachdb/sql-foundations (found keywords: SQL statement)

If we have not gotten back to your issue within a few business days, you can try the following:

  • Join our community slack channel and ask on #cockroachdb.
  • Try find someone from here if you know they worked closely on the area and CC them.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-triaged blathers was able to find an owner labels Feb 23, 2024
@koorosh
Copy link
Collaborator

koorosh commented Mar 29, 2024

Some additional context:

  1. It appears that part of index creation statement is built on FE:
    // createIdxName creates an index name, roughly following
    // Postgres's conventions for naming anonymous indexes.
    // For example:
    //
    // CREATE INDEX ON t (a)
    // => t_a_rec_idx
    //
    // CREATE INDEX ON t ((a + b), c, lower(d))
    // => t_expr_c_expr1_rec_idx
    //
    // Adding an extra _rec to the name, so we can identify statistics on indexes
    // that were creating using this feature.
    export function createIdxName(statement: string): string {
  2. Index recommendation from crdb_internal.cluster_execution_insights suggests an index name with . (dots) which is not allowed if not wrapped with quotes.
    Screenshot 2024-03-29 at 15 03 20
  3. Could reproduce similar issue when suggested index should be created on table with name like "t2.1"
    Screenshot 2024-03-29 at 15 46 07

Assume that logic on UI should be reduced to consume CREATE INDEX... query as is from the server.

abarganier added a commit to abarganier/cockroach that referenced this issue Apr 3, 2024
Fixes: cockroachdb#119579

The UI code used to modify the index recommendations provided by the
server was not properly accounting for table names containing quotation
marks. This was causing invalid CREATE INDEX statements to be generated,
which would fail on request.

This patch fixes this by updating the query modification code to strip
quotation marks from the table name prior to using it to generate an
index name.

Release note (bug fix): Index recommendations in the DB Console will now
function properly for tables whose names are in quotation marks.
craig bot pushed a commit that referenced this issue Apr 10, 2024
121731: ui: index recommendations properly handle quoted table names r=dhartunian a=abarganier

Fixes: #119579

Epic: none

The UI code used to modify the index recommendations provided by the
server was not properly accounting for table and column names containing
quotation marks. This was causing invalid CREATE INDEX statements to
be generated, which would fail on request.

This patch fixes this by updating the query modification code to strip
quotation marks from the table name prior to using it to generate an
index name.

Release note (bug fix): Index recommendations in the DB Console will now
function properly for indexes on tables/columns whose names contain
quotation marks and/or whitespace.
For example: `CREATE INDEX ON "my table" ("my col");`

122058: roachtest: improve acceptance test running time, use suite r=renatolabs,srosenberg a=rickystewart

Closes #118513.

Epic: None
Release note: None

122060: go.mod: bump Pebble to 6cdb88d44473 r=RaduBerinde a=RaduBerinde

Changes:
 * [`6cdb88d4`](cockroachdb/pebble@6cdb88d4) sstable: clean up UpdateKeySuffixes

We update the Cockroach code to implement the new method.

Release note: none.
Epic: none.

Co-authored-by: Alex Barganier <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Radu Berinde <[email protected]>
@craig craig bot closed this as completed in 8398e91 Apr 10, 2024
blathers-crl bot pushed a commit that referenced this issue Apr 10, 2024
Fixes: #119579

The UI code used to modify the index recommendations provided by the
server was not properly accounting for table and column names containing
quotation marks. This was causing invalid CREATE INDEX statements to
be generated, which would fail on request.

This patch fixes this by updating the query modification code to strip
quotation marks from the table name prior to using it to generate an
index name.

Release note (bug fix): Index recommendations in the DB Console will now
function properly for indexes on tables/columns whose names contain
quotation marks and/or whitespace.
For example: `CREATE INDEX ON "my table" ("my col");`
blathers-crl bot pushed a commit that referenced this issue Apr 10, 2024
Fixes: #119579

The UI code used to modify the index recommendations provided by the
server was not properly accounting for table and column names containing
quotation marks. This was causing invalid CREATE INDEX statements to
be generated, which would fail on request.

This patch fixes this by updating the query modification code to strip
quotation marks from the table name prior to using it to generate an
index name.

Release note (bug fix): Index recommendations in the DB Console will now
function properly for indexes on tables/columns whose names contain
quotation marks and/or whitespace.
For example: `CREATE INDEX ON "my table" ("my col");`
blathers-crl bot pushed a commit that referenced this issue Apr 10, 2024
Fixes: #119579

The UI code used to modify the index recommendations provided by the
server was not properly accounting for table and column names containing
quotation marks. This was causing invalid CREATE INDEX statements to
be generated, which would fail on request.

This patch fixes this by updating the query modification code to strip
quotation marks from the table name prior to using it to generate an
index name.

Release note (bug fix): Index recommendations in the DB Console will now
function properly for indexes on tables/columns whose names contain
quotation marks and/or whitespace.
For example: `CREATE INDEX ON "my table" ("my col");`
abarganier added a commit that referenced this issue May 9, 2024
Fixes: #119579

The UI code used to modify the index recommendations provided by the
server was not properly accounting for table and column names containing
quotation marks. This was causing invalid CREATE INDEX statements to
be generated, which would fail on request.

This patch fixes this by updating the query modification code to strip
quotation marks from the table name prior to using it to generate an
index name.

Release note (bug fix): Index recommendations in the DB Console will now
function properly for indexes on tables/columns whose names contain
quotation marks and/or whitespace.
For example: `CREATE INDEX ON "my table" ("my col");`
abarganier added a commit that referenced this issue May 9, 2024
Fixes: #119579

The UI code used to modify the index recommendations provided by the
server was not properly accounting for table and column names containing
quotation marks. This was causing invalid CREATE INDEX statements to
be generated, which would fail on request.

This patch fixes this by updating the query modification code to strip
quotation marks from the table name prior to using it to generate an
index name.

Release note (bug fix): Index recommendations in the DB Console will now
function properly for indexes on tables/columns whose names contain
quotation marks and/or whitespace.
For example: `CREATE INDEX ON "my table" ("my col");`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-observability-inf C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community T-observability X-blathers-triaged blathers was able to find an owner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants