Skip to content

Commit b39295a

Browse files
craig[bot]rafissyuzefovich
committed
152309: sql: fix sequence dependencies for IDENTITY columns in pg_depend r=rafiss a=rafiss fixes #108439 Release note (bug fix): pg_class.pg_depend now contains entries with deptype='i' (internal) for identity columns that depend on sequences. These previously had deptype='a' (auto). 152481: parserutils: extract shared PopulateErrorDetails into new package r=yuzefovich a=yuzefovich This allows us to break the dependency from `jsonpath/parser` on `sql/parser`. Additionally, it fixes the bug where we used the hardcoded (inherited from `sql/parser`) code for ERROR token in other parsers - the method now explicitly takes that code as an argument. Informs: #79357. Epic: None Release note: None 152537: cli/interactive_tests: use interrupt_and_wait helper everywhere r=rafiss a=rafiss This helper was added in 799f5ab to reduce flakiness by waiting for process termination. This patch applies the same change to other tests that benefit. fixes #152482 Release note: None 152538: roachtest: mark more activerecord tests as flaky r=rafiss a=rafiss fixes #152405 fixes #151710 Release note: None Co-authored-by: Rafi Shamim <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]>
5 parents c9daf32 + b2ad4a6 + abca1ec + d9722f0 + b5967b2 commit b39295a

File tree

25 files changed

+255
-142
lines changed

25 files changed

+255
-142
lines changed

‎pkg/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,7 @@ ALL_TESTS = [
771771
"//pkg/util/json/tokenizer:tokenizer_test",
772772
"//pkg/util/json:json_disallowed_imports_test",
773773
"//pkg/util/json:json_test",
774+
"//pkg/util/jsonpath/parser:parser_disallowed_imports_test",
774775
"//pkg/util/jsonpath/parser:parser_test",
775776
"//pkg/util/limit:limit_test",
776777
"//pkg/util/log/eventlog:eventlog_test",
@@ -2164,6 +2165,7 @@ GO_TARGETS = [
21642165
"//pkg/sql/parser/statements:statements",
21652166
"//pkg/sql/parser:parser",
21662167
"//pkg/sql/parser:parser_test",
2168+
"//pkg/sql/parserutils:parserutils",
21672169
"//pkg/sql/pgrepl/lsn:lsn",
21682170
"//pkg/sql/pgrepl/lsnutil:lsnutil",
21692171
"//pkg/sql/pgrepl/pgreplparser:pgreplparser",

‎pkg/cli/interactive_tests/test_encryption.tcl‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ end_test
4545
start_test "Start normal node."
4646
send "$argv start-single-node --insecure --store=$storedir\r"
4747
eexpect "node starting"
48-
interrupt
49-
eexpect "shutdown completed"
48+
interrupt_and_wait
5049
send "$argv debug encryption-status $storedir\r"
5150
eexpect ""
5251
end_test
@@ -59,22 +58,19 @@ end_test
5958
start_test "Restart with plaintext."
6059
send "$argv start-single-node --insecure --store=$storedir --enterprise-encryption=path=$storedir,key=plain,old-key=plain\r"
6160
eexpect "node starting"
62-
interrupt
63-
eexpect "shutdown completed"
61+
interrupt_and_wait
6462
send "$argv debug encryption-status $storedir --enterprise-encryption=path=$storedir,key=plain,old-key=plain\r"
6563
eexpect " \"Active\": true,\r\n \"Type\": \"Plaintext\","
6664
# Try starting without the encryption flag.
6765
send "$argv start-single-node --insecure --store=$storedir\r"
6866
eexpect "node starting"
69-
interrupt
70-
eexpect "shutdown completed"
67+
interrupt_and_wait
7168
end_test
7269

7370
start_test "Restart with AES-128."
7471
send "$argv start-single-node --insecure --store=$storedir --enterprise-encryption=path=$storedir,key=$keydir/aes-128.key,old-key=plain\r"
7572
eexpect "node starting"
76-
interrupt
77-
eexpect "shutdown completed"
73+
interrupt_and_wait
7874
file_not_exists "$storedir/COCKROACHDB_REGISTRY"
7975
send "$argv debug encryption-status $storedir --enterprise-encryption=path=$storedir,key=$keydir/aes-128.key,old-key=plain\r"
8076
eexpect " \"Active\": true,\r\n \"Type\": \"AES128_CTR\","
@@ -89,14 +85,13 @@ end_test
8985
start_test "Restart with AES-256."
9086
send "$argv start-single-node --insecure --store=$storedir --enterprise-encryption=path=$storedir,key=$keydir/aes-256.key,old-key=$keydir/aes-128.key\r"
9187
eexpect "node starting"
92-
interrupt
93-
eexpect "shutdown completed"
88+
interrupt_and_wait
9489
send "$argv debug encryption-status $storedir --enterprise-encryption=path=$storedir,key=$keydir/aes-256.key,old-key=plain\r"
9590
eexpect " \"Active\": true,\r\n \"Type\": \"AES256_CTR\","
9691
# Startup again, but don't specify the old key, it's no longer in use.
9792
send "$argv start-single-node --insecure --store=$storedir --enterprise-encryption=path=$storedir,key=$keydir/aes-256.key,old-key=plain\r"
9893
eexpect "node starting"
99-
interrupt
94+
interrupt_and_wait
10095
# Try starting without the encryption flag.
10196
send "$argv start-single-node --insecure --store=$storedir\r"
10297
eexpect "encryption was used on this store before, but no encryption flags specified."
@@ -116,6 +111,5 @@ end_test
116111
start_test "Run with WAL failover path."
117112
send "$argv start-single-node --insecure --store=$storedir --wal-failover=path=$failoverdir --enterprise-encryption=path=$storedir,key=$keydir/aes-256.key,old-key=$keydir/aes-128.key --enterprise-encryption=path=$failoverdir,key=$keydir/aes-256.key,old-key=$keydir/aes-128.key\r"
118113
eexpect "node starting"
119-
interrupt
120-
eexpect "shutdown completed"
114+
interrupt_and_wait
121115
end_test

‎pkg/cli/interactive_tests/test_flags.tcl‎

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,19 @@ eexpect ":/# "
99
start_test "Check that --max-disk-temp-storage works."
1010
send "$argv start-single-node --insecure --store=path=logs/mystore --max-disk-temp-storage=10GiB\r"
1111
eexpect "node starting"
12-
interrupt
13-
eexpect ":/# "
12+
interrupt_and_wait
1413
end_test
1514

1615
start_test "Check that --max-disk-temp-storage can be expressed as a percentage."
1716
send "$argv start-single-node --insecure --store=path=logs/mystore --max-disk-temp-storage=10%\r"
1817
eexpect "node starting"
19-
interrupt
20-
eexpect ":/# "
18+
interrupt_and_wait
2119
end_test
2220

2321
start_test "Check that --max-disk-temp-storage percentage works when the store is in-memory."
2422
send "$argv start-single-node --insecure --store=type=mem,size=1GB --max-disk-temp-storage=10%\r"
2523
eexpect "node starting"
26-
interrupt
27-
eexpect ":/# "
24+
interrupt_and_wait
2825
end_test
2926

3027
start_test "Check that memory max flags do not exceed available RAM."
@@ -34,16 +31,14 @@ eexpect "is larger than"
3431
eexpect "of total RAM"
3532
eexpect "increased risk"
3633
eexpect "node starting"
37-
interrupt
38-
eexpect ":/# "
34+
interrupt_and_wait
3935
end_test
4036

4137
start_test "Check that not using --host nor --advertise causes a user warning."
4238
send "$argv start-single-node --insecure\r"
4339
eexpect "WARNING: neither --listen-addr nor --advertise-addr was specified"
4440
eexpect "node starting"
45-
interrupt
46-
eexpect ":/# "
41+
interrupt_and_wait
4742
end_test
4843

4944
start_test "Check that using --advertise-addr does not cause a user warning."
@@ -55,16 +50,14 @@ expect {
5550
}
5651
}
5752
eexpect "node starting"
58-
interrupt
59-
eexpect ":/# "
53+
interrupt_and_wait
6054
end_test
6155

6256
start_test "Check that --listening-url-file gets created with the right data"
6357
send "$argv start-single-node --insecure --listening-url-file=foourl\r"
6458
eexpect "node starting"
6559
system "grep -q 'postgresql://.*@.*:\[0-9\]\[0-9\]*' foourl"
66-
interrupt
67-
eexpect ":/# "
60+
interrupt_and_wait
6861
end_test
6962

7063
start_test {Check that the "failed running SUBCOMMAND" message does not consider a flag the subcommand}
@@ -109,8 +102,7 @@ end_test
109102
start_test "Check that locality flags without a region tier warn"
110103
send "$argv start-single-node --insecure --locality=data-center=us-east,zone=a\r"
111104
eexpect "WARNING: The --locality flag does not contain a"
112-
interrupt
113-
eexpect ":/# "
105+
interrupt_and_wait
114106
end_test
115107

116108
start_server $argv
@@ -170,8 +162,7 @@ send "export GOMEMLIMIT=1GiB;\r"
170162
eexpect ":/# "
171163
send "$argv start-single-node --insecure --store=path=logs/mystore\r"
172164
eexpect "node starting"
173-
interrupt
174-
eexpect ":/# "
165+
interrupt_and_wait
175166
stop_server $argv
176167
end_test
177168

@@ -180,8 +171,7 @@ send "export GOGC=500;\r"
180171
eexpect ":/# "
181172
send "$argv start-single-node --insecure --store=path=logs/mystore\r"
182173
eexpect "node starting"
183-
interrupt
184-
eexpect ":/# "
174+
interrupt_and_wait
185175
stop_server $argv
186176
end_test
187177

‎pkg/cli/interactive_tests/test_missing_log_output.tcl‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@ eexpect ":/# "
3333
# is has been broken, and that may be the secondary logger.
3434
send "tail -F `find logs/db/logs -type l`\r"
3535
eexpect "error: write */dev/stderr*: broken pipe"
36-
interrupt
37-
eexpect ":/# "
36+
interrupt_and_wait
3837
end_test
3938

4039
start_test "Check that a broken log file prints a message to stderr."
@@ -52,8 +51,7 @@ eexpect "CockroachDB node starting"
5251
end_test
5352

5453
# Stop it.
55-
interrupt
56-
eexpect ":/# "
54+
interrupt_and_wait
5755

5856
# Disable replication so as to avoid spurious purgatory errors.
5957
start_server $argv
@@ -68,9 +66,7 @@ eexpect "CockroachDB node starting"
6866
end_test
6967

7068
# Stop it.
71-
interrupt
72-
interrupt
73-
eexpect ":/# "
69+
interrupt_and_wait
7470
system "rm -rf logs/db"
7571

7672
start_test "Check that --logtostderr can override the threshold but no error is printed on startup"
@@ -79,9 +75,7 @@ eexpect "marker\r\nCockroachDB node starting"
7975
end_test
8076

8177
# Stop it.
82-
interrupt
83-
interrupt
84-
eexpect ":/# "
78+
interrupt_and_wait
8579
system "rm -rf logs/db"
8680

8781

‎pkg/cli/interactive_tests/test_socket_name.tcl‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,12 @@ system "test -r .s.PGSQL.$sql_port.lock"
5050
end_test
5151

5252
# Stop the server that was started above.
53-
interrupt
54-
eexpect ":/# "
53+
interrupt_and_wait
5554
send "exit\r"
5655
eexpect eof
5756

5857
set spawn_id $shell1_spawn_id
59-
interrupt
60-
eexpect ":/# "
58+
interrupt_and_wait
6159

6260
start_test "Check that the socket-dir flag checks the length of the directory."
6361
send "$argv start-single-node --insecure --socket-dir=$longname\r"

‎pkg/cli/interactive_tests/test_sql_mem_monitor.tcl‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,7 @@ expect {
9797
timeout { handle_timeout "memory allocation error" }
9898
}
9999
# Stop the tail command.
100-
interrupt
101-
eexpect ":/# "
100+
interrupt_and_wait
102101

103102
# Check that the client got a bad connection error
104103
set spawn_id $client_spawn_id
@@ -137,8 +136,6 @@ send_eof
137136
eexpect eof
138137

139138
set spawn_id $shell_spawn_id
140-
interrupt
141-
interrupt
142-
eexpect ":/# "
139+
interrupt_and_wait
143140
send "exit\r"
144141
eexpect eof

‎pkg/cli/interactive_tests/test_temp_dir.tcl‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,7 @@ if {! [string match "$cwd/$tempdir/$tempprefix*" [gets $rfile]]} {
6262
exit 1
6363
}
6464
close $rfile
65-
interrupt
66-
eexpect "shutdown completed"
65+
interrupt_and_wait
6766
# Verify the temp directory is removed.
6867
glob_not_exists "$tempdir/$tempprefix*"
6968
# Verify temp directory path is removed from record file.
@@ -80,8 +79,7 @@ eexpect "node starting"
8079
eexpect "temp dir:*$tempdir/$tempprefix"
8180
# Verify the temp directory under first store is created.
8281
glob_exists "$tempdir/$tempprefix*"
83-
interrupt
84-
eexpect "shutdown completed"
82+
interrupt_and_wait
8583
# Verify the temp directory is removed.
8684
glob_not_exists "$tempdir/$tempprefix*"
8785
end_test
@@ -97,8 +95,7 @@ eexpect "temp dir:*$cwd/$tempdir/$tempprefix"
9795
# Verify temp1 and temp2 are removed shortly after startup.
9896
file_not_exists "$storedir/temp1"
9997
file_not_exists "$storedir/temp2"
100-
interrupt
101-
eexpect "shutdown completed"
98+
interrupt_and_wait
10299
# Verify the temp directory is removed.
103100
glob_not_exists "$tempdir/$tempprefix*"
104101
# Verify store directory still exists.
@@ -111,8 +108,7 @@ eexpect "node starting"
111108
eexpect "temp dir:*$cwd/$storedir/$tempprefix"
112109
# Verify the temp directory under first store is created.
113110
glob_exists "$storedir/$tempprefix*"
114-
interrupt
115-
eexpect "shutdown completed"
111+
interrupt_and_wait
116112
# Verify the temp directory is removed.
117113
glob_not_exists "$storedir/$tempprefix*"
118114
# Verify the store file still exists.
@@ -131,6 +127,5 @@ send "pkill -9 cockroach\r"
131127
# We should be able to start the cockroach instance again.
132128
send "$argv start-single-node --insecure --store=$storedir\r"
133129
eexpect "node starting"
134-
interrupt
135-
eexpect "shutdown completed"
130+
interrupt_and_wait
136131
end_test

‎pkg/cmd/roachtest/tests/activerecord_blocklist.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ var activeRecordIgnoreList = blocklist{
2929
`ActiveRecord::ConnectionAdapters::PostgreSQLAdapterTest#test_translate_no_connection_exception_to_not_established`: "pg_terminate_backend not implemented",
3030
`ActiveRecord::Encryption::EncryptableRecordTest#test_by_default,_it's_case_sensitive`: "flaky",
3131
`ActiveRecord::Encryption::EncryptableRecordTest#test_forced_encoding_for_deterministic_attributes_will_replace_invalid_characters`: "flaky",
32+
`ActiveRecord::Encryption::UniquenessValidationsTest#test_uniqueness_validations_work_when_using_old_encryption_schemes`: "flaky",
3233
`AssociationCallbacksTest#test_has_many_callbacks_for_destroy_on_parent`: "flaky",
3334
`BasicsTest#test_default_values_are_deeply_dupped`: "flaky",
3435
`CascadedEagerLoadingTest#test_eager_association_loading_with_cascaded_three_levels_by_ping_pong`: "flaky",
@@ -37,6 +38,14 @@ var activeRecordIgnoreList = blocklist{
3738
`CockroachDB::AdapterForeignKeyTest#test_foreign_key_violations_on_insert_are_translated_to_specific_exception`: "flaky",
3839
`CockroachDB::FixturesTest#test_create_fixtures`: "flaky",
3940
`FixtureWithSetModelClassPrevailsOverNamingConventionTest#test_model_class_in_fixture_file_is_respected`: "flaky",
41+
`HasManyAssociationsTest#test_collection_association_with_private_kernel_method`: "flaky",
42+
`HasManyAssociationsTest#test_delete_all_association_with_primary_key_deletes_correct_records`: "flaky",
43+
`HasManyAssociationsTest#test_dependence`: "flaky",
44+
`HasManyAssociationsTest#test_dependence_on_account`: "flaky",
45+
`HasManyAssociationsTest#test_dependent_association_respects_optional_conditions_on_delete`: "flaky",
46+
`HasManyAssociationsTest#test_dependent_association_respects_optional_hash_conditions_on_delete`: "flaky",
47+
`HasManyAssociationsTest#test_dependent_association_respects_optional_sanitized_conditions_on_delete`: "flaky",
48+
`HasManyAssociationsTest#test_depends_and_nullify`: "flaky",
4049
`InheritanceTest#test_eager_load_belongs_to_primary_key_quoting`: "flaky",
4150
`InheritanceTest#test_eager_load_belongs_to_something_inherited`: "flaky",
4251
`PostgresqlArrayTest#test_uniqueness_validation`: "affected by autocommit_before_ddl",

‎pkg/sql/logictest/testdata/logic_test/orms‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,3 +435,71 @@ and traint.conparentid = 0 ORDER BY traint.conrelid, traint.conname
435435
----
436436
public b public a false b_a_id_fkey {"(a_id,id)"} false
437437
public src public dst false src_d_fkey {"(d,a)"} false
438+
439+
# Test for issue #108439: EFCore introspection query for IDENTITY columns
440+
# needs pg_depend entries with deptype 'i'.
441+
442+
statement ok
443+
CREATE TABLE efcore_identity_test (
444+
id INT8 NOT NULL GENERATED BY DEFAULT AS IDENTITY,
445+
CONSTRAINT pk_testtable PRIMARY KEY (id ASC)
446+
)
447+
448+
query TTTTTTTBTTTTTTTBTTIIIIBI colnames
449+
SELECT
450+
nspname,
451+
cls.relname,
452+
typ.typname,
453+
basetyp.typname AS basetypname,
454+
attname,
455+
description,
456+
collname,
457+
attisdropped,
458+
attidentity::TEXT,
459+
attgenerated::TEXT,
460+
''::text as attcompression,
461+
format_type(typ.oid, atttypmod) AS formatted_typname,
462+
format_type(basetyp.oid, typ.typtypmod) AS formatted_basetypname,
463+
CASE
464+
WHEN pg_proc.proname = 'array_recv' THEN 'a'
465+
ELSE typ.typtype
466+
END AS typtype,
467+
CASE WHEN pg_proc.proname='array_recv' THEN elemtyp.typname END AS elemtypname,
468+
NOT (attnotnull OR typ.typnotnull) AS nullable,
469+
CASE
470+
WHEN atthasdef THEN (SELECT pg_get_expr(adbin, cls.oid) FROM pg_attrdef WHERE adrelid = cls.oid AND adnum = attr.attnum)
471+
END AS default,
472+
-- Sequence options for identity columns
473+
format_type(seqtypid, 0) AS seqtype, seqstart, seqmin, seqmax, seqincrement, seqcycle, seqcache
474+
FROM pg_class AS cls
475+
JOIN pg_namespace AS ns ON ns.oid = cls.relnamespace
476+
LEFT JOIN pg_attribute AS attr ON attrelid = cls.oid
477+
LEFT JOIN pg_type AS typ ON attr.atttypid = typ.oid
478+
LEFT JOIN pg_proc ON pg_proc.oid = typ.typreceive
479+
LEFT JOIN pg_type AS elemtyp ON (elemtyp.oid = typ.typelem)
480+
LEFT JOIN pg_type AS basetyp ON (basetyp.oid = typ.typbasetype)
481+
LEFT JOIN pg_description AS des ON des.objoid = cls.oid AND des.objsubid = attnum
482+
LEFT JOIN pg_collation as coll ON coll.oid = attr.attcollation
483+
-- Bring in identity sequences the depend on this column.
484+
LEFT JOIN pg_depend AS dep ON dep.refobjid = cls.oid AND dep.refobjsubid = attr.attnum AND dep.deptype = 'i'
485+
LEFT JOIN pg_sequence AS seq ON seq.seqrelid = dep.objid
486+
WHERE
487+
cls.relkind IN ('r', 'v', 'm', 'f') AND
488+
nspname NOT IN ('pg_catalog', 'information_schema', 'crdb_internal') AND
489+
attnum > 0 AND
490+
cls.relname = 'efcore_identity_test' AND
491+
NOT EXISTS (
492+
SELECT 1 FROM pg_depend WHERE
493+
classid=(
494+
SELECT cls.oid
495+
FROM pg_class AS cls
496+
JOIN pg_namespace AS ns ON ns.oid = cls.relnamespace
497+
WHERE relname='pg_class' AND ns.nspname='pg_catalog'
498+
) AND
499+
objid=cls.oid AND
500+
deptype IN ('e', 'x')
501+
)
502+
ORDER BY attnum;
503+
----
504+
nspname relname typname basetypname attname description collname attisdropped attidentity attgenerated attcompression formatted_typname formatted_basetypname typtype elemtypname nullable default seqtype seqstart seqmin seqmax seqincrement seqcycle seqcache
505+
public efcore_identity_test int8 NULL id NULL NULL false d · · bigint NULL b NULL false nextval('public.efcore_identity_test_id_seq'::REGCLASS) bigint 1 1 9223372036854775807 1 false 1

0 commit comments

Comments
 (0)