Skip to content

Commit

Permalink
sql: refactor DISCARD so it runs during execution
Browse files Browse the repository at this point in the history
Previously, it would run during planning, which is not correct, or at
least, makes it harder to reason about what's going on.

Release note: None

Release justification: low risk refactor
  • Loading branch information
rafiss committed Aug 21, 2022
1 parent 42b67ff commit 04bf2f3
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 17 deletions.
29 changes: 20 additions & 9 deletions pkg/sql/discard.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,28 +22,39 @@ import (
// Discard implements the DISCARD statement.
// See https://www.postgresql.org/docs/9.6/static/sql-discard.html for details.
func (p *planner) Discard(ctx context.Context, s *tree.Discard) (planNode, error) {
switch s.Mode {
return &discardNode{mode: s.Mode}, nil
}

type discardNode struct {
mode tree.DiscardMode
}

func (n *discardNode) Next(_ runParams) (bool, error) { return false, nil }
func (n *discardNode) Values() tree.Datums { return nil }
func (n *discardNode) Close(_ context.Context) {}
func (n *discardNode) startExec(params runParams) error {
switch n.mode {
case tree.DiscardModeAll:
if !p.autoCommit {
return nil, pgerror.New(pgcode.ActiveSQLTransaction,
if !params.p.autoCommit {
return pgerror.New(pgcode.ActiveSQLTransaction,
"DISCARD ALL cannot run inside a transaction block")
}

// RESET ALL
if err := p.sessionDataMutatorIterator.applyOnEachMutatorError(
if err := params.p.sessionDataMutatorIterator.applyOnEachMutatorError(
func(m sessionDataMutator) error {
return resetSessionVars(ctx, m)
return resetSessionVars(params.ctx, m)
},
); err != nil {
return nil, err
return err
}

// DEALLOCATE ALL
p.preparedStatements.DeleteAll(ctx)
params.p.preparedStatements.DeleteAll(params.ctx)
default:
return nil, errors.AssertionFailedf("unknown mode for DISCARD: %d", s.Mode)
return errors.AssertionFailedf("unknown mode for DISCARD: %d", n.mode)
}
return newZeroNode(nil /* columns */), nil
return nil
}

func resetSessionVars(ctx context.Context, m sessionDataMutator) error {
Expand Down
8 changes: 1 addition & 7 deletions pkg/sql/pgwire/testdata/pgtest/pgjdbc
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,7 @@ send
Query {"String": "DISCARD ALL"}
----

until crdb_only ignore=ParameterStatus
ReadyForQuery
----
{"Type":"CommandComplete","CommandTag":"DISCARD"}
{"Type":"ReadyForQuery","TxStatus":"I"}

until noncrdb_only ignore=ParameterStatus
until ignore=ParameterStatus ignore=NoticeResponse
ReadyForQuery
----
{"Type":"CommandComplete","CommandTag":"DISCARD ALL"}
Expand Down
8 changes: 7 additions & 1 deletion pkg/sql/sem/tree/stmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,13 @@ func (*Discard) StatementReturnType() StatementReturnType { return Ack }
func (*Discard) StatementType() StatementType { return TypeTCL }

// StatementTag returns a short string identifying the type of statement.
func (*Discard) StatementTag() string { return "DISCARD" }
func (d *Discard) StatementTag() string {
switch d.Mode {
case DiscardModeAll:
return "DISCARD ALL"
}
return "DISCARD"
}

// StatementReturnType implements the Statement interface.
func (n *DeclareCursor) StatementReturnType() StatementReturnType { return Ack }
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ var planNodeNames = map[reflect.Type]string{
reflect.TypeOf(&delayedNode{}): "virtual table",
reflect.TypeOf(&deleteNode{}): "delete",
reflect.TypeOf(&deleteRangeNode{}): "delete range",
reflect.TypeOf(&discardNode{}): "discard",
reflect.TypeOf(&distinctNode{}): "distinct",
reflect.TypeOf(&dropDatabaseNode{}): "drop database",
reflect.TypeOf(&dropExternalConnectionNode{}): "drop external connection",
Expand Down

0 comments on commit 04bf2f3

Please sign in to comment.