Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
114499: backupccl: add some basic checks for online restore r=msbutler a=stevendanna

This adds some basic guardrails to stop people from trying things out
that currently don't work.

Epic: none

Release note: None

114550: parser: allow a_expr in SET ON UPDATE clause r=rafiss a=rafiss

This was preventing some statements from round-tripping the parse-format-parse cycle. Note that this grammar change matches the expressions we allow for DEFAULT expressions.

fixes #114480
Release note: None

114551: roachtest: lower connection timeout and include node ID r=rafiss a=rafiss

This adds two improvements to help with debugging:
- Include the node ID in the message if we fail to connect to a node.
- Fix the connection timeout at 1 minute rather than leaving it infinite.

informs #114501
Release note: None

Co-authored-by: Steven Danna <danna@cockroachlabs.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
  • Loading branch information
3 people committed Nov 16, 2023
4 parents c748278 + 2c546af + 323800d + e325b60 commit f47f05c
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 5 deletions.
2 changes: 1 addition & 1 deletion docs/generated/sql/bnf/alter_table_cmds.bnf
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
alter_table_cmds ::=
( ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_new_name | 'RENAME' 'CONSTRAINT' constraint_name 'TO' constraint_new_name | 'ADD' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ADD' 'COLUMN' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'ON' 'UPDATE' b_expr | 'DROP' 'ON' 'UPDATE' ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'VISIBLE' | 'SET' 'NOT' 'VISIBLE' ) | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) ( 'NOT' 'VALID' | ) | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem ( 'NOT' 'VALID' | ) | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' ( 'USING' 'HASH' | ) ( 'WITH' '(' ( ( ( storage_parameter_key '=' value ) ) ( ( ',' ( storage_parameter_key '=' value ) ) )* ) ')' ) | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' ( 'READ' 'WRITE' | 'OFF' ) | ( ( 'PARTITION' 'BY' ( 'LIST' '(' name_list ')' '(' list_partitions ')' | 'RANGE' '(' name_list ')' '(' range_partitions ')' | 'NOTHING' ) ) | 'PARTITION' 'ALL' 'BY' ( 'LIST' '(' name_list ')' '(' list_partitions ')' | 'RANGE' '(' name_list ')' '(' range_partitions ')' | 'NOTHING' ) ) | 'SET' '(' ( ( ( storage_parameter_key '=' value ) ) ( ( ',' ( storage_parameter_key '=' value ) ) )* ) ')' | 'RESET' '(' ( ( storage_parameter_key ) ( ( ',' storage_parameter_key ) )* ) ')' ) ) ( ( ',' ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_new_name | 'RENAME' 'CONSTRAINT' constraint_name 'TO' constraint_new_name | 'ADD' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ADD' 'COLUMN' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'ON' 'UPDATE' b_expr | 'DROP' 'ON' 'UPDATE' ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'VISIBLE' | 'SET' 'NOT' 'VISIBLE' ) | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) ( 'NOT' 'VALID' | ) | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem ( 'NOT' 'VALID' | ) | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' ( 'USING' 'HASH' | ) ( 'WITH' '(' ( ( ( storage_parameter_key '=' value ) ) ( ( ',' ( storage_parameter_key '=' value ) ) )* ) ')' ) | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' ( 'READ' 'WRITE' | 'OFF' ) | ( ( 'PARTITION' 'BY' ( 'LIST' '(' name_list ')' '(' list_partitions ')' | 'RANGE' '(' name_list ')' '(' range_partitions ')' | 'NOTHING' ) ) | 'PARTITION' 'ALL' 'BY' ( 'LIST' '(' name_list ')' '(' list_partitions ')' | 'RANGE' '(' name_list ')' '(' range_partitions ')' | 'NOTHING' ) ) | 'SET' '(' ( ( ( storage_parameter_key '=' value ) ) ( ( ',' ( storage_parameter_key '=' value ) ) )* ) ')' | 'RESET' '(' ( ( storage_parameter_key ) ( ( ',' storage_parameter_key ) )* ) ')' ) ) )*
( ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_new_name | 'RENAME' 'CONSTRAINT' constraint_name 'TO' constraint_new_name | 'ADD' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ADD' 'COLUMN' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'ON' 'UPDATE' a_expr | 'DROP' 'ON' 'UPDATE' ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'VISIBLE' | 'SET' 'NOT' 'VISIBLE' ) | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) ( 'NOT' 'VALID' | ) | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem ( 'NOT' 'VALID' | ) | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' ( 'USING' 'HASH' | ) ( 'WITH' '(' ( ( ( storage_parameter_key '=' value ) ) ( ( ',' ( storage_parameter_key '=' value ) ) )* ) ')' ) | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' ( 'READ' 'WRITE' | 'OFF' ) | ( ( 'PARTITION' 'BY' ( 'LIST' '(' name_list ')' '(' list_partitions ')' | 'RANGE' '(' name_list ')' '(' range_partitions ')' | 'NOTHING' ) ) | 'PARTITION' 'ALL' 'BY' ( 'LIST' '(' name_list ')' '(' list_partitions ')' | 'RANGE' '(' name_list ')' '(' range_partitions ')' | 'NOTHING' ) ) | 'SET' '(' ( ( ( storage_parameter_key '=' value ) ) ( ( ',' ( storage_parameter_key '=' value ) ) )* ) ')' | 'RESET' '(' ( ( storage_parameter_key ) ( ( ',' storage_parameter_key ) )* ) ')' ) ) ( ( ',' ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_new_name | 'RENAME' 'CONSTRAINT' constraint_name 'TO' constraint_new_name | 'ADD' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ADD' 'COLUMN' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename ( ( ) ( ( col_qualification ) )* ) ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'ON' 'UPDATE' a_expr | 'DROP' 'ON' 'UPDATE' ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'VISIBLE' | 'SET' 'NOT' 'VISIBLE' ) | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) ( 'NOT' 'VALID' | ) | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem ( 'NOT' 'VALID' | ) | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' ( 'USING' 'HASH' | ) ( 'WITH' '(' ( ( ( storage_parameter_key '=' value ) ) ( ( ',' ( storage_parameter_key '=' value ) ) )* ) ')' ) | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' ( 'READ' 'WRITE' | 'OFF' ) | ( ( 'PARTITION' 'BY' ( 'LIST' '(' name_list ')' '(' list_partitions ')' | 'RANGE' '(' name_list ')' '(' range_partitions ')' | 'NOTHING' ) ) | 'PARTITION' 'ALL' 'BY' ( 'LIST' '(' name_list ')' '(' list_partitions ')' | 'RANGE' '(' name_list ')' '(' range_partitions ')' | 'NOTHING' ) ) | 'SET' '(' ( ( ( storage_parameter_key '=' value ) ) ( ( ',' ( storage_parameter_key '=' value ) ) )* ) ')' | 'RESET' '(' ( ( storage_parameter_key ) ( ( ',' storage_parameter_key ) )* ) ')' ) ) )*
2 changes: 1 addition & 1 deletion docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -4289,7 +4289,7 @@ alter_column_default ::=
| 'DROP' 'DEFAULT'

alter_column_on_update ::=
'SET' 'ON' 'UPDATE' b_expr
'SET' 'ON' 'UPDATE' a_expr
| 'DROP' 'ON' 'UPDATE'

alter_column_visible ::=
Expand Down
41 changes: 41 additions & 0 deletions pkg/ccl/backupccl/restore_online.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"fmt"
"time"

"github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backuppb"
"github.com/cockroachdb/cockroach/pkg/jobs"
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
Expand All @@ -21,6 +22,8 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
"github.com/cockroachdb/cockroach/pkg/util/log"
Expand Down Expand Up @@ -189,3 +192,41 @@ func sendAddRemoteSSTWorker(
return flush(nil)
}
}

// checkManifestsForOnlineCompat returns an error if the set of
// manifests appear to be from a backup that we cannot currently
// support for online restore.
func checkManifestsForOnlineCompat(ctx context.Context, manifests []backuppb.BackupManifest) error {
if len(manifests) < 1 {
return errors.AssertionFailedf("expected at least 1 backup manifest")
}
// TODO(online-restore): Remove once we support layer ordering.
if len(manifests) > 1 {
return pgerror.Newf(pgcode.FeatureNotSupported, "experimental online restore: restoring from an incremental backup not supported")
}

// TODO(online-restore): Remove once we support layer ordering and have tested some reasonable number of layers.
const layerLimit = 16
if len(manifests) > layerLimit {
return pgerror.Newf(pgcode.FeatureNotSupported, "experimental online restore: too many incremental layers %d (from backup) > %d (limit)", len(manifests), layerLimit)
}

for _, manifest := range manifests {
if !manifest.RevisionStartTime.IsEmpty() || !manifest.StartTime.IsEmpty() || manifest.MVCCFilter == backuppb.MVCCFilter_All {
return pgerror.Newf(pgcode.FeatureNotSupported, "experimental online restore: restoring from a revision history backup not supported")
}
}
return nil
}

// checkRewritesAreNoops returns an error if any of the rewrites in
// the rewrite map actually require key rewriting. We currently don't
// rewrite keys, so this would be a problem.
func checkRewritesAreNoops(rewrites jobspb.DescRewriteMap) error {
for oldID, rw := range rewrites {
if rw.ID != oldID {
return pgerror.Newf(pgcode.FeatureNotSupported, "experimental online restore: descriptor rewrites not supported but required (%d -> %d)", oldID, rw.ID)
}
}
return nil
}
41 changes: 41 additions & 0 deletions pkg/ccl/backupccl/restore_online_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,47 @@ func TestOnlineRestoreBasic(t *testing.T) {
bankOnlineRestore(t, rSQLDB, numAccounts, externalStorage)
}

func TestOnlineRestoreErrors(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

_, sqlDB, dir, cleanupFn := backupRestoreTestSetup(t, singleNode, 1, InitManualReplication)
defer cleanupFn()
params := base.TestClusterArgs{}
_, rSQLDB, cleanupFnRestored := backupRestoreTestSetupEmpty(t, 1, dir, InitManualReplication, params)
defer cleanupFnRestored()
rSQLDB.Exec(t, "CREATE DATABASE data")
var (
fullBackup = "nodelocal://1/full-backup"
fullBackupWithRevs = "nodelocal://1/full-backup-with-revs"
incrementalBackup = "nodelocal://1/incremental-backup"
incrementalBackupWithRevs = "nodelocal://1/incremental-backup-with-revs"
)

t.Run("incremental backups are unsupported", func(t *testing.T) {
sqlDB.Exec(t, fmt.Sprintf("BACKUP INTO '%s'", incrementalBackup))
sqlDB.Exec(t, fmt.Sprintf("BACKUP INTO LATEST IN '%s'", incrementalBackup))
rSQLDB.ExpectErr(t, "incremental backup not supported", fmt.Sprintf("RESTORE TABLE data.bank FROM LATEST IN '%s' WITH EXPERIMENTAL DEFERRED COPY", incrementalBackup))
})
t.Run("full backups with revision history are unsupported", func(t *testing.T) {
var systemTime string
sqlDB.QueryRow(t, "SELECT cluster_logical_timestamp()").Scan(&systemTime)
sqlDB.Exec(t, fmt.Sprintf("BACKUP INTO '%s' AS OF SYSTEM TIME '%s' WITH revision_history", fullBackupWithRevs, systemTime))
rSQLDB.ExpectErr(t, "revision history backup not supported", fmt.Sprintf("RESTORE TABLE data.bank FROM LATEST IN '%s' WITH EXPERIMENTAL DEFERRED COPY", fullBackupWithRevs))
})
t.Run("icremental backups with revision history are unsupported", func(t *testing.T) {
sqlDB.Exec(t, fmt.Sprintf("BACKUP INTO '%s' WITH revision_history", incrementalBackupWithRevs))
sqlDB.Exec(t, fmt.Sprintf("BACKUP INTO LATEST IN '%s' WITH revision_history", incrementalBackupWithRevs))
rSQLDB.ExpectErr(t, "incremental backup not supported", fmt.Sprintf("RESTORE TABLE data.bank FROM LATEST IN '%s' WITH EXPERIMENTAL DEFERRED COPY", incrementalBackupWithRevs))
})
t.Run("descriptor rewrites are unsupported", func(t *testing.T) {
sqlDB.Exec(t, fmt.Sprintf("BACKUP INTO '%s'", fullBackup))
rSQLDB.Exec(t, "CREATE DATABASE new_data")
rSQLDB.ExpectErr(t, "descriptor rewrites not supported", fmt.Sprintf("RESTORE TABLE data.bank FROM LATEST IN '%s' WITH into_db=new_data,EXPERIMENTAL DEFERRED COPY", fullBackup))
})

}

func bankOnlineRestore(
t *testing.T, sqlDB *sqlutils.SQLRunner, numAccounts int, externalStorage string,
) {
Expand Down
20 changes: 19 additions & 1 deletion pkg/ccl/backupccl/restore_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -1361,6 +1361,12 @@ func restorePlanHook(
}
}

if restoreStmt.Options.ExperimentalOnline && !restoreStmt.Targets.TenantID.IsSet() {
// TODO(ssd): Disable this once it is less annoying to
// disable it in tests.
log.Warningf(ctx, "running non-tenant online RESTORE; this is dangerous and will only work if you know exactly what you are doing")
}

var newTenantID *roachpb.TenantID
var newTenantName *roachpb.TenantName
if restoreStmt.Options.AsTenant != nil || restoreStmt.Options.ForceTenantID != nil {
Expand Down Expand Up @@ -1856,10 +1862,15 @@ func doRestorePlan(
err = checkBackupManifestVersionCompatability(ctx, p.ExecCfg().Settings.Version,
mainBackupManifests, restoreStmt.Options.UnsafeRestoreIncompatibleVersion)
if err != nil {

return err
}

if restoreStmt.Options.ExperimentalOnline {
if err := checkManifestsForOnlineCompat(ctx, mainBackupManifests); err != nil {
return err
}
}

if restoreStmt.DescriptorCoverage == tree.AllDescriptors {
// Validate that the backup is a full cluster backup if a full cluster restore was requested.
if mainBackupManifests[0].DescriptorCoverage == tree.RequestedDescriptors {
Expand Down Expand Up @@ -2122,6 +2133,13 @@ func doRestorePlan(
} else {
fromDescription = from
}

if restoreStmt.Options.ExperimentalOnline {
if err := checkRewritesAreNoops(descriptorRewrites); err != nil {
return err
}
}

description, err := restoreJobDescription(
ctx,
p,
Expand Down
7 changes: 6 additions & 1 deletion pkg/cmd/roachtest/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -2539,7 +2539,9 @@ func (c *clusterImpl) Conn(
// ConnE returns a SQL connection to the specified node.
func (c *clusterImpl) ConnE(
ctx context.Context, l *logger.Logger, node int, opts ...func(*option.ConnOption),
) (*gosql.DB, error) {
) (_ *gosql.DB, retErr error) {
// NB: errors.Wrap returns nil if err is nil.
defer func() { retErr = errors.Wrapf(retErr, "connecting to node %d", node) }()

connOptions := &option.ConnOption{}
for _, opt := range opts {
Expand Down Expand Up @@ -2569,6 +2571,9 @@ func (c *clusterImpl) ConnE(
for k, v := range connOptions.Options {
vals.Add(k, v)
}
// connect_timeout is a libpq-specific parameter for the maximum wait for
// connection, in seconds.
vals.Add("connect_timeout", "60")
dataSourceName = dataSourceName + "&" + vals.Encode()
}
db, err := gosql.Open("postgres", dataSourceName)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/parser/sql.y
Original file line number Diff line number Diff line change
Expand Up @@ -2930,7 +2930,7 @@ alter_column_default:
}

alter_column_on_update:
SET ON UPDATE b_expr
SET ON UPDATE a_expr
{
$$.val = $4.expr()
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/parser/testdata/alter_table
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,14 @@ ALTER TABLE a ALTER COLUMN b SET ON UPDATE (current_timestamp()) -- fully parent
ALTER TABLE a ALTER COLUMN b SET ON UPDATE current_timestamp() -- literals removed
ALTER TABLE _ ALTER COLUMN _ SET ON UPDATE current_timestamp() -- identifiers removed

parse
ALTER TABLE a ALTER b SET ON UPDATE c IS NOT DISTINCT FROM FALSE
----
ALTER TABLE a ALTER COLUMN b SET ON UPDATE c IS false -- normalized!
ALTER TABLE a ALTER COLUMN b SET ON UPDATE ((c) IS (false)) -- fully parenthesized
ALTER TABLE a ALTER COLUMN b SET ON UPDATE c IS NOT DISTINCT FROM _ -- literals removed
ALTER TABLE _ ALTER COLUMN _ SET ON UPDATE _ IS false -- identifiers removed

parse
ALTER TABLE a ALTER COLUMN b DROP ON UPDATE
----
Expand Down

0 comments on commit f47f05c

Please sign in to comment.