sql: add errorIfRows node #37101
sql: add errorIfRows node #37101
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis, @justinj, @RaduBerinde, and @yuzefovich)
pkg/sql/error_if_rows.go, line 48 at r1 (raw file):
if ok { // TODO(yuzefovich): what should the error be here? return false, pgerror.Newf(pgerror.CodeForeignKeyViolationError, "foreign key violation")
I think you will need to have the node inspect the row to create the error. Look at fk_existence_batch.go and see how they do it. You will probably want this node to accept a function that it can use at runtime to turn the row into an error, or something like that.
There was a problem hiding this comment.
Reviewable status:
pkg/sql/error_if_rows.go, line 48 at r1 (raw file):
Previously, jordanlewis (Jordan Lewis) wrote…
I think you will need to have the node inspect the row to create the error. Look at
fk_existence_batch.goand see how they do it. You will probably want this node to accept a function that it can use at runtime to turn the row into an error, or something like that.
Yeah, we will want to parametrize the error message somehow (maybe the optimizer would pass down a string with %s for the values) . Sample messages from the fk logictest:
foreign key violation: value \[5\] not found in customer reviews@primary \[id\]
foreign key violation: values \[2\] in columns \[id\] referenced in table "review_stats"
We could also not worry about it for now and revisit later.
Add errorIfRows node that returns an error if it receives a row from its input. It is needed for foreign keys checks implementation with joins. Release note: None
yuzefovich
force-pushed
the
error-if-rows
branch
from
b7cc403
to
1ce7045
Compare
|
@jordanlewis are you ok with merging this as is and modifying later as needed? |
|
Yes. LGTM. |
|
TFTRs! bors r+ |
37101: sql: add errorIfRows node r=yuzefovich a=yuzefovich Add errorIfRows node that returns an error if it receives a row from its input. It is needed for foreign keys checks implementation with joins. Fixes: #37053. I'm not sure whether some of these additions are necessary or if something is missing. Also, I'm not sure whether it deserves a unit test. Release note: None 37130: sql: fix checking for presence of window functions in some edge cases r=yuzefovich a=yuzefovich Previously, when a window function was a part of another expression (say BinaryExpr) instead of being used directly within PARTITION BY and ORDER BY clauses of another window function, we would incorrectly not reject such a query and would actually crash while trying to execute it. Release note: None Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Build succeeded |
Add errorIfRows node that returns an error if it receives a row
from its input. It is needed for foreign keys checks implementation
with joins.
Fixes: #37053.
I'm not sure whether some of these additions are necessary or if something is missing. Also, I'm not sure whether it deserves a unit test.
Release note: None