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

tree: fix error message for parsing tuples in arrays #63995

Closed
rafiss opened this issue Apr 21, 2021 · 0 comments · Fixed by #71916
Closed

tree: fix error message for parsing tuples in arrays #63995

rafiss opened this issue Apr 21, 2021 · 0 comments · Fixed by #71916
Assignees
@rafiss
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@rafiss
Copy link
Collaborator

rafiss commented Apr 21, 2021 •
edited
Loading

See the following error. It can be resolved by adding a case to ParseAndRequireString for TupleFamily. This might also resolve the Sentry report in #60494

root@:26257/defaultdb> select '{(1,2)}'::record[];

ERROR: internal error: could not parse "{(1,2)}" as type tuple[]: unknown type tuple (*types.T)
SQLSTATE: XX000
DETAIL: stack trace:
/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/parse_string.go:87: ParseAndRequireString()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/parse_array.go:140: parseElement()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/parse_array.go:188: doParseDArrayFromString()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/parse_array.go:159: ParseDArrayFromString()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/casts.go:1021: PerformCast()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/eval.go:3726: Eval()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/values.go:155: startExec()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/plan.go:506: func2()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/walk.go:119: func1()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/walk.go:298: visitInternal()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/walk.go:86: visit()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/walk.go:50: walkPlan()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/plan.go:509: startExec()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/plan_node_to_row_source.go:125: Start()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/execinfra/processorsbase.go:774: Run()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/flowinfra/flow.go:393: Run()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:417: Run()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:997: PlanAndRun()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1001: execWithDistSQLEngine()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:872: dispatchToExecutionEngine()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:639: execStmtInOpenState()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:114: execStmt()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1465: func1()

OR we could do what postgres does and have a nicer failure for this

rafiss@127:postgres>  select '{(1,2)}'::record[];
input of anonymous composite types is not implemented
LINE 1: select '{(1,2)}'::record[]

rafiss@127:postgres>  select '{(1,2)}'::record(int,int)[];
type modifier is not allowed for type "record[]"
LINE 1: select '{(1,2)}'::record(int,int)[]
@rafiss rafiss added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Apr 21, 2021
@rafiss rafiss self-assigned this Apr 21, 2021
@rafiss rafiss mentioned this issue Apr 21, 2021
Merged
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Sep 14, 2021
@rafiss rafiss changed the title tree: add support for parsing tuples in arrays tree: fix error message for parsing tuples in arrays Sep 16, 2021
@rafiss rafiss mentioned this issue Sep 16, 2021
Closed
craig bot pushed a commit that referenced this issue Sep 20, 2021
@rafiss @rimadeodhar @andreimatei
Merge #63996 #68082 #69484
bc4fc6e 
63996: rowenc: add value encoding for arrays of tuples r=yuzefovich a=rafiss

fixes #32715
resurrecting an old PR #32871

Previously, queries that required an array of tuples to be encoded would
fail. This could only happen if a distsql query required such a datum to
be sent over the wire. This commit adds an encoding so such queries can
succeed.

A good followup task to do after this is #63995

Release note (bug fix): queries involving arrays of tuples will no
longer spuriously fail due to an encoding error.

68082: RFC: Update metrics framework for CRDB. r=rimadeodhar a=rimadeodhar

This RFC covers the the updates needed to the metrics
framework to convert and expose metric aggregations
as "indicators". The RFC introduces the term "indicators"
to be defined as a meaningful aggregation of one or more
metrics to provide insights into the health of the system.

It discusses the overall design for a framework to define
indicators from a metric and expose through an HTTP endpoint
using an introspection API. The goal of this RFC is to
provide a mechanism to better document metrics as indicators
and provide engineering with a template to specify how metrics
should be used for monitoring and debugging.

Release note: None

69484: contextutil: record actual duration in TimeoutError r=andreimatei a=andreimatei

Before this patch, a TimeoutError would report what the timeout was, but
not how long the operation actually ran for. This is confusing when the
operation actually ran for much longer than the timeout (because nobody
was watching the ctx). This patch makes the error also report the actual
duration.

Release note: None

Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: rimadeodhar <rima@cockroachlabs.com>
Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
@rafiss rafiss mentioned this issue Oct 25, 2021
Merged
@craig craig bot closed this as completed in f6caf2d Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rafiss rafiss

Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql/sem/tree: support parsing of tuples from string literals rafiss/cockroach
1 participant
@rafiss