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.

Sign up for GitHub

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

Jump to bottom

roachtest: sql: error parsing sql statment for crdb_internal.hide_sql_constants #96555

Closed
cockroach-teamcity opened this issue Feb 5, 2023 · 3 comments · Fixed by #96899
Closed

roachtest: sql: error parsing sql statment for crdb_internal.hide_sql_constants #96555

cockroach-teamcity opened this issue Feb 5, 2023 · 3 comments · Fixed by #96899
Assignees
@xinhaoz
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot.
Milestone
23.1

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Feb 5, 2023 •
edited by cockroach-jira-scripts
Loading

roachtest.sqlsmith/setup=seed/setting=no-mutations failed with artifacts on master @ 8e24570fa366ed038c6ae65f50db5d8e22826db0:

test artifacts and logs in: /artifacts/sqlsmith/setup=seed/setting=no-mutations/run_1
(sqlsmith.go:238).func3: error: pq: internal error: crdb_internal.hide_sql_constants(): expected 1 statement, but found 0
stmt:
SELECT
	tab226._jsonb AS "c'�ol \r610",
	'-13 years -5 mons -533 days -02:56:21.507089':::INTERVAL AS "%vco%0bl611",
	crdb_internal.hide_sql_constants(e'\r':::STRING::STRING)::STRING AS co🙄l612,
	tab226._bytes AS col613,
	(-2.4993441104888916):::FLOAT8 AS col614
FROM
	defaultdb.public.seed@seed__int8__float8__date_idx AS tab226
ORDER BY
	tab226._timestamptz NULLS FIRST;

Parameters: ROACHTEST_cloud=gce , ROACHTEST_cpu=4 , ROACHTEST_encrypted=false , ROACHTEST_ssd=0

Help

See: roachtest README

See: How To Investigate (internal)

/cc @cockroachdb/sql-queries

This test on roachdash | Improve this report!

Jira issue: CRDB-24184
@cockroach-teamcity cockroach-teamcity added branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. labels Feb 5, 2023
@cockroach-teamcity cockroach-teamcity added this to the 23.1 milestone Feb 5, 2023
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Feb 5, 2023
@rharding6373
Copy link
Collaborator

rharding6373 commented Feb 6, 2023 •
edited
Loading

Repro:

SELECT crdb_internal.hide_sql_constants(e'\r':::STRING);

Stack trace:

ERROR: internal error: crdb_internal.hide_sql_constants(): expected 1 statement, but found 0
SQLSTATE: XX000
DETAIL: stack trace:
github.com/cockroachdb/cockroach/pkg/sql/parser/parse.go:156: parseOneWithInt()
github.com/cockroachdb/cockroach/pkg/sql/parser/parse.go:317: ParseOneWithInt()
github.com/cockroachdb/cockroach/pkg/sql/parser/parse.go:310: ParseOne()
github.com/cockroachdb/cockroach/pkg/sql/sem/builtins/builtins.go:7713: func328()
github.com/cockroachdb/cockroach/pkg/sql/sem/builtins/builtins.go:8901: func1()
github.com/cockroachdb/cockroach/pkg/sql/colexec/builtin_funcs.go:77: func1()
github.com/cockroachdb/cockroach/pkg/sql/colmem/allocator.go:441: PerformOperation()
github.com/cockroachdb/cockroach/pkg/sql/colexec/builtin_funcs.go:54: Next()
github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecutils/operator.go:137: Next()
github.com/cockroachdb/cockroach/bazel-out/darwin-fastbuild/bin/pkg/sql/colexec/colexecbase/const.eg.go:407: Next()
github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecbase/simple_project.go:124: Next()
github.com/cockroachdb/cockroach/pkg/sql/colflow/stats.go:118: next()
github.com/cockroachdb/cockroach/pkg/sql/colexecerror/error.go:92: CatchVectorizedRuntimeError()
github.com/cockroachdb/cockroach/pkg/sql/colflow/stats.go:126: Next()
github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:252: nextAdapter()
github.com/cockroachdb/cockroach/pkg/sql/colexecerror/error.go:92: CatchVectorizedRuntimeError()
github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:256: next()
github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:294: Run()
github.com/cockroachdb/cockroach/pkg/sql/colflow/vectorized_flow.go:309: Run()
github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:857: Run()
github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1793: PlanAndRun()
github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1544: PlanAndRunAll()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1642: execWithDistSQLEngine()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1263: dispatchToExecutionEngine()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:745: execStmtInOpenState()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:131: func1()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:2530: execWithProfiling()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:130: execStmt()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1969: func1()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1974: execCmd()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1892: run()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:861: ServeConn()

@rharding6373 rharding6373 changed the title roachtest: sqlsmith/setup=seed/setting=no-mutations failed roachtest: sql: error parsing sql statment for crdb_internal.hide_sql_constants Feb 6, 2023
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Feb 7, 2023
@michae2
Copy link
Collaborator

michae2 commented Feb 7, 2023

[sql queries triage meeting] SQL Sessions: we will remove this function from sqlsmith, but seems like it shouldn't panic, is this yours?

rharding6373 added a commit to rharding6373/cockroach that referenced this issue Feb 7, 2023
@rharding6373
sqlsmith: disable crdb_internal.hide_sql_constants
0529c92 
Informs: cockroachdb#96555
Epic: None
@rharding6373 rharding6373 mentioned this issue Feb 7, 2023
Merged
craig bot pushed a commit that referenced this issue Feb 7, 2023
@otan @rharding6373
Merge #96572 #96741
60c6293 
96572: parser: no-op if QUOTE '"' is specified in COPY r=otan a=otan

Release note (sql change): `COPY ... FROM ... QUOTE '"'` will no longer error.

Epic: None

Informs: #85574

Release justification: fixes usability issues with third party tools that is really a no-op.

96741: sqlsmith: disable crdb_internal.hide_sql_constants r=rharding6373 a=rharding6373

Informs: #96555
Epic: None

Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
Co-authored-by: rharding6373 <rharding6373@users.noreply.github.com>
@rafiss
Copy link
Collaborator

rafiss commented Feb 9, 2023

The @cockroachdb/sql-observability team added this function; so I will assign them.

@rafiss rafiss removed the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Feb 9, 2023
@michae2 michae2 removed the T-sql-queries SQL Queries Team label Feb 9, 2023
@xinhaoz xinhaoz self-assigned this Feb 9, 2023
@xinhaoz xinhaoz mentioned this issue Feb 9, 2023
Merged
craig bot pushed a commit that referenced this issue Feb 10, 2023
@rafiss @xinhaoz
Merge #96892 #96899 #96958
5f310bc 
96892: cacheutil: fix computation of entry size r=ZhouXing19 a=rafiss

fixes #89444

The previous way would always result in a fixed size for each entry, since it would just compute the size of a pointer. Now, the size of the entry must be specified when writing to the cache.

Release note: None

96899: builtins: crdb_internal.hide_sql_constants should not error on parse r=xinhaoz a=xinhaoz

Previously, if crdb_internal.hide_sql_constants is unable to parse the provided string as a sql statement, it returned an error. Instead, we should just return the empty string to make it less disruptive.

Epic: none
Fixes: #96555

Release note: None

96958: roachtest: rename SQLExperience -> SQLSessions r=jordanlewis a=rafiss

Epic: None
Release note: None

Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: Xin Hao Zhang <xzhang@cockroachlabs.com>
@craig craig bot closed this as completed in 43374b7 Feb 10, 2023
michae2 added a commit to michae2/cockroach that referenced this issue Mar 1, 2023
@michae2
builtins: fix error case of crdb_internal.hide_sql_constants
9818625 
Fix one of the error cases of `hide_sql_constants` to append an empty
string to the array, rather than returning an empty string. Also, add
`hide_sql_constants` back to sqlsmith now that cockroachdb#96555 is fixed.

Epic: None

Fixes: cockroachdb#97830

Release note: None
@michae2 michae2 mentioned this issue Mar 1, 2023
Merged
michae2 added a commit to michae2/cockroach that referenced this issue Mar 3, 2023
@michae2
sqlsmith: bring back hide_sql_constants
247abc4 
Now that cockroachdb#96555 and cockroachdb#97830 are fixed, add `hide_sql_constants` back to
sqlsmith. Also add some missing spaces to the info strings.

Epic: None

Release note: None
craig bot pushed a commit that referenced this issue Mar 3, 2023
@michae2
Merge #97831
007f794 
97831: sqlsmith: bring back hide_sql_constants r=rharding6373 a=michae2

Now that #96555 and #97830 are fixed, add `hide_sql_constants` back to sqlsmith. Also add some missing spaces to the info strings.

Epic: None

Release note: None

Co-authored-by: Michael Erickson <michae2@cockroachlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@xinhaoz xinhaoz

Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot.
Projects
None yet
Milestone
23.1
Development

Successfully merging a pull request may close this issue.

builtins: crdb_internal.hide_sql_constants should not error on parse xinhaoz/cockroach
5 participants
@rafiss @rharding6373 @xinhaoz @cockroach-teamcity @michae2