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

sql: json minus (RemoveIndex) does not work properly #35612

Closed
knz opened this issue Mar 11, 2019 · 1 comment · Fixed by #35617
Closed

sql: json minus (RemoveIndex) does not work properly #35612

knz opened this issue Mar 11, 2019 · 1 comment · Fixed by #35617
Assignees
@justinj
Labels
A-sql-builtins SQL built-in functions and semantics thereof. A-sql-json JSON handling in SQL. A-sql-optimizer SQL logical planning and optimizations. A-sql-pgcompat Semantic compatibility with PostgreSQL C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@knz
Copy link
Contributor

knz commented Mar 11, 2019

psql:

kena=> select '[123]'::jsonb - 0;
 ?column?
----------
 []

crdb:

[email protected]:17034/defaultdb> select '[123]'::json - 0;
  ?column?
+----------+
  [123]

@mjibson @justinj ?
@knz knz added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-json JSON handling in SQL. A-sql-builtins SQL built-in functions and semantics thereof. labels Mar 11, 2019
@knz knz added the A-sql-optimizer SQL logical planning and optimizations. label Mar 11, 2019
@knz
Copy link
Contributor Author

knz commented Mar 11, 2019

I suspect (expr *BinaryExpr) normalize() in sem/tree/normalize.go.

justinj pushed a commit to justinj/cockroach that referenced this issue Mar 11, 2019
opt,sql: don't fold "- 0" for JSON
ffcf9fc 
Fixes cockroachdb#35612.

Prior to this commit, we would unconditionally fold x - 0 to x. This was
invalid in the case where x was JSON, since `- 0` in that case means
"delete the 0th entry in the array".

This bug existed in both the HP and CBO.

This fix slightly abuses the existing "types.IsAdditiveType" function to
determine whether or not the identity applies.

Release note (bug fix): Subtracting 0 from a JSON array now correctly
removes its first element.
@justinj justinj mentioned this issue Mar 11, 2019
Merged
justinj pushed a commit to justinj/cockroach that referenced this issue Mar 11, 2019
opt,sql: don't fold "- 0" for JSON
1aa4e56 
Fixes cockroachdb#35612.

Prior to this commit, we would unconditionally fold x - 0 to x. This was
invalid in the case where x was JSON, since `- 0` in that case means
"delete the 0th entry in the array".

This bug existed in both the HP and CBO.

This fix slightly abuses the existing "types.IsAdditiveType" function to
determine whether or not the identity applies.

Release note (bug fix): Subtracting 0 from a JSON array now correctly
removes its first element.
craig bot pushed a commit that referenced this issue Mar 11, 2019
Merge #35617
1877438 
35617: opt,sql: don't fold "- 0" for JSON r=justinj a=justinj

Fixes #35612.

Prior to this commit, we would unconditionally fold x - 0 to x. This was
invalid in the case where x was JSON, since `- 0` in that case means
"delete the 0th entry in the array".

This bug existed in both the HP and CBO.

This fix slightly abuses the existing "types.IsAdditiveType" function to
determine whether or not the identity applies.

Release note (bug fix): Subtracting 0 from a JSON array now correctly
removes its first element.

Co-authored-by: Justin Jaffray <[email protected]>
justinj pushed a commit to justinj/cockroach that referenced this issue Mar 11, 2019
opt,sql: don't fold "- 0" for JSON
6d0d73f 
This was harder than I thought! I forgot how much the opt codebase has
changed since 2.1.

Fixes cockroachdb#35612.

Prior to this commit, we would unconditionally fold x - 0 to x. This was
invalid in the case where x was JSON, since `- 0` in that case means
"delete the 0th entry in the array".

This bug existed in both the HP and CBO.

This fix slightly abuses the existing "types.IsAdditiveType" function to
determine whether or not the identity applies.

Release note (bug fix): Subtracting 0 from a JSON array now correctly
removes its first element.
@craig craig bot closed this as completed in #35617 Mar 11, 2019
@justinj justinj mentioned this issue Mar 11, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@justinj justinj

Labels
A-sql-builtins SQL built-in functions and semantics thereof. A-sql-json JSON handling in SQL. A-sql-optimizer SQL logical planning and optimizations. A-sql-pgcompat Semantic compatibility with PostgreSQL C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

opt,sql: don't fold "- 0" for JSON justinj/cockroach
2 participants
@justinj @knz