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

sql: implement IFERROR and ISERROR #25304

Merged
merged 1 commit into from May 16, 2018
Merged

Conversation

knz
Copy link
Contributor

@knz knz commented May 3, 2018

Fixes #9760.

The SQL scalar operators IFERROR and ISERROR are inspired from the
MSSQL feature of the same name. In their simple form:

  • ISERROR(Expr) returns false if Expr evaluates
    without error; true otherwise.
  • IFERROR(Expr, Val) returns the value of Expr if Expr evaluates
    without error, otherwise it evaluates and returns Val.

In order to avoid catching too many errors, they exist also in an
"advanced" mode:

  • ISERROR(Expr, Code) returns false if Expr evaluates without
    error; otherwise, it evaluates Code, and if Code evaluates
    without error it checks whether the error produced for Expr has pg
    error code that matches Code. If Code is NULL or the pg error
    code matches, ISERROR returns true, otherwise ISERROR returns
    Expr's evaluation error. For example, `ISERROR(1/x, '22012')
    returns true iff Expr evaluates with a "divide by zero" error.

  • IFERROR(Expr, Val, Code) evaluates Expr and returns its value if
    no error. If there is an error, it evaluates Code and checks the
    Expr eval error with Code as for ISERROR above. If the error
    code matches, it evaluates and returns Val.

Note: this only covers the evaluation of scalar expressions that
do not require access to data in the underlying KV store.

  • If ISERROR/IFERROR is applied to a subquery and the subquery errors
    out, the entire query fails and ISERROR/IFERROR are ineffective.
  • In general if the scalar expression causes a KV operation and the KV
    operation fails (e.g. nextval() on a sequence), again the entire
    query fails.
  • Some very specific uses of KV may be caught (e.g. ::REGCLASS casts)
    but this is neither tested nor guaranteed.

Release note (sql change): the two experimental scalar operators
IFERROR() and ISERROR() have been introduced. They may be
documented for public use in the future.

@knz knz requested a review from nvanbenschoten May 3, 2018 23:15
@knz knz added this to Triage in (DEPRECATED) SQL Front-end, Lang & Semantics via automation May 3, 2018
@knz knz requested review from a team May 3, 2018 23:15
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz
Copy link
Contributor Author

knz commented May 3, 2018

cc @andreimatei

@knz
Copy link
Contributor Author

knz commented May 4, 2018

Note: I made a mistake in the definition of iserror -- my explanation about "either/or code" is wrong.

@knz knz moved this from Triage to Current milestone in (DEPRECATED) SQL Front-end, Lang & Semantics May 4, 2018
@knz
Copy link
Contributor Author

knz commented May 4, 2018

Fixed, PTAL

@nvanbenschoten
Copy link
Member

Reviewed 9 of 9 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


pkg/sql/sem/tree/expr.go, line 542 at r1 (raw file):

}

// IfErrExpr represents an IFERR expression.

s/IFERR/IFERROR/


pkg/sql/sem/tree/expr.go, line 546 at r1 (raw file):

	Cond    Expr
	Else    Expr
	ErrCond Expr

I would rename ErrCond to ErrCode everywhere. It's a little confusing having multiple "condition" expressions within the same parent Expr.


pkg/sql/sem/tree/expr.go, line 579 at r1 (raw file):

}

// IsErrExpr represents an IFERR expression.

s/IFERR/ISERROR/


pkg/sql/sem/tree/expr.go, line 580 at r1 (raw file):

// IsErrExpr represents an IFERR expression.
type IsErrExpr struct {

Do you think it's worth folding this into IfErrExpr with a nil Else expression? Should be pretty easy to make that work.


pkg/sql/sem/tree/type_check.go, line 621 at r1 (raw file):

	var typedErrCond TypedExpr
	if expr.ErrCond != nil {
		typedErrCond, err = typeCheckAndRequire(ctx, expr.ErrCond, types.String, "IFERROR")

s/IFERROR/ISERROR/


Comments from Reviewable

@knz
Copy link
Contributor Author

knz commented May 4, 2018

Review status: all files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.


pkg/sql/sem/tree/expr.go, line 542 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

s/IFERR/IFERROR/

Done.


pkg/sql/sem/tree/expr.go, line 546 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

I would rename ErrCond to ErrCode everywhere. It's a little confusing having multiple "condition" expressions within the same parent Expr.

Done.


pkg/sql/sem/tree/expr.go, line 579 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

s/IFERR/ISERROR/

Done.


pkg/sql/sem/tree/expr.go, line 580 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Do you think it's worth folding this into IfErrExpr with a nil Else expression? Should be pretty easy to make that work.

do you mean that iferror(a) should type as boolean whereas iferror(a,b) should type as the type of a and b? That looks fishy. I think we've been pretty disciplined at having 1 expr type === 1 typing rule.


pkg/sql/sem/tree/type_check.go, line 621 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

s/IFERROR/ISERROR/

Done.


Comments from Reviewable

@nvanbenschoten
Copy link
Member

Review status: 4 of 9 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


pkg/sql/sem/tree/expr.go, line 580 at r1 (raw file):

Previously, knz (kena) wrote…

do you mean that iferror(a) should type as boolean whereas iferror(a,b) should type as the type of a and b? That looks fishy. I think we've been pretty disciplined at having 1 expr type === 1 typing rule.

No sorry, I just meant folding these two expression types into a single AST node.


Comments from Reviewable

@knz knz force-pushed the 20180504-iferr branch 2 times, most recently from 409fcb8 to 0acdf3d Compare May 9, 2018 18:46
@knz
Copy link
Contributor Author

knz commented May 9, 2018

Review status: 4 of 9 files reviewed at latest revision, 1 unresolved discussion.


pkg/sql/sem/tree/expr.go, line 580 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

No sorry, I just meant folding these two expression types into a single AST node.

Done. RFAL


Comments from Reviewable

@knz knz force-pushed the 20180504-iferr branch 3 times, most recently from ef40e39 to fb16165 Compare May 15, 2018 16:04
@nvanbenschoten
Copy link
Member

:lgtm:


Reviewed 6 of 6 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


Comments from Reviewable

The SQL scalar operators `IFERROR` and `ISERROR` are inspired from the
MSSQL feature of the same name. In their simple form:

- `ISERROR(Expr)` returns false if `Expr` evaluates
  without error; true otherwise.
- `IFERROR(Expr, Val)` returns the value of `Expr` if `Expr` evaluates
  without error, otherwise it evaluates and returns `Val`.

In order to avoid catching too many errors, they exist also in an
"advanced" mode:

- `ISERROR(Expr, Code)` returns false if `Expr` evaluates without
  error; otherwise, it evaluates `Code`, and if `Code` evaluates
  without error it checks whether the error produced for `Expr` has pg
  error code that matches `Code`. If `Code` is NULL or the pg error
  code matches, `ISERROR` returns true, otherwise `ISERROR` returns
  `Expr`'s evaluation error. For example, `ISERROR(1/x, '22012')
  returns true iff Expr evaluates with a "divide by zero" error.

- `IFERROR(Expr, Val, Code)` evaluates `Expr` and returns its value if
  no error. If there is an error, it evaluates `Code` and checks the
  `Expr` eval error with `Code` as for `ISERROR` above. If the error
  code matches, it evaluates and returns `Val`.

Note: this only covers the evaluation of scalar expressions that
do not require access to data in the underlying KV store.

- If ISERROR/IFERROR is applied to a subquery and the subquery errors
  out, the entire query fails and ISERROR/IFERROR are ineffective.
- In general if the scalar expression causes a KV operation and the KV
  operation fails (e.g. `nextval()` on a sequence), again the entire
  query fails.
- Some very specific uses of KV may be caught (e.g. ::REGCLASS casts)
  but this is neither tested nor guaranteed.

Release note (sql change): the two experimental scalar operators
`IFERROR()` and `ISERROR()` have been introduced. They may be
documented for public use in the future.
@knz
Copy link
Contributor Author

knz commented May 16, 2018

Thanks!

bors r+

craig bot pushed a commit that referenced this pull request May 16, 2018
25304: sql: implement IFERROR and ISERROR r=knz a=knz

Fixes #9760.

The SQL scalar operators `IFERROR` and `ISERROR` are inspired from the
MSSQL feature of the same name. In their simple form:

- `ISERROR(Expr)` returns false if `Expr` evaluates
  without error; true otherwise.
- `IFERROR(Expr, Val)` returns the value of `Expr` if `Expr` evaluates
  without error, otherwise it evaluates and returns `Val`.

In order to avoid catching too many errors, they exist also in an
"advanced" mode:

- `ISERROR(Expr, Code)` returns false if `Expr` evaluates without
  error; otherwise, it evaluates `Code`, and if `Code` evaluates
  without error it checks whether the error produced for `Expr` has pg
  error code that matches `Code`. If `Code` is NULL or the pg error
  code matches, `ISERROR` returns true, otherwise `ISERROR` returns
  `Expr`'s evaluation error. For example, `ISERROR(1/x, '22012')
  returns true iff Expr evaluates with a "divide by zero" error.

- `IFERROR(Expr, Val, Code)` evaluates `Expr` and returns its value if
  no error. If there is an error, it evaluates `Code` and checks the
  `Expr` eval error with `Code` as for `ISERROR` above. If the error
  code matches, it evaluates and returns `Val`.

Note: this only covers the evaluation of scalar expressions that
do not require access to data in the underlying KV store.

- If ISERROR/IFERROR is applied to a subquery and the subquery errors
  out, the entire query fails and ISERROR/IFERROR are ineffective.
- In general if the scalar expression causes a KV operation and the KV
  operation fails (e.g. `nextval()` on a sequence), again the entire
  query fails.
- Some very specific uses of KV may be caught (e.g. ::REGCLASS casts)
  but this is neither tested nor guaranteed.

Release note (sql change): the two experimental scalar operators
`IFERROR()` and `ISERROR()` have been introduced. They may be
documented for public use in the future.


Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
@craig
Copy link
Contributor

craig bot commented May 16, 2018

Build succeeded

@craig craig bot merged commit e50b039 into cockroachdb:master May 16, 2018
@knz knz moved this from Current milestone to Finished (milestone 0423) in (DEPRECATED) SQL Front-end, Lang & Semantics May 17, 2018
@knz knz deleted the 20180504-iferr branch July 2, 2018 09:29
justinj pushed a commit to justinj/cockroach that referenced this pull request Dec 8, 2018
This commit introduces the IFERROR/ISERROR scalar operators. See cockroachdb#25304
for more background on these operators.

Release note: None
justinj pushed a commit to justinj/cockroach that referenced this pull request Dec 10, 2018
This commit introduces the IFERROR/ISERROR scalar operators. See cockroachdb#25304
for more background on these operators.

Release note: None
justinj pushed a commit to justinj/cockroach that referenced this pull request Dec 11, 2018
This commit introduces the IFERROR/ISERROR scalar operators. See cockroachdb#25304
for more background on these operators.

Release note: None
justinj pushed a commit to justinj/cockroach that referenced this pull request Dec 13, 2018
This commit introduces the IFERROR/ISERROR scalar operators. See cockroachdb#25304
for more background on these operators.

Release note: None
craig bot pushed a commit that referenced this pull request Dec 13, 2018
32957: opt: add support for IFERROR/ISERROR r=justinj a=justinj

This commit introduces the IFERROR/ISERROR scalar operators. See #25304
for more background on these operators.

Release note: None

Co-authored-by: Justin Jaffray <justin@cockroachlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants