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

parquet: decoder should handle empty bit(0) arrays #137757

Open
cockroach-teamcity opened this issue Dec 19, 2024 · 8 comments
Open

parquet: decoder should handle empty bit(0) arrays #137757

cockroach-teamcity opened this issue Dec 19, 2024 · 8 comments
Labels
A-cdc Change Data Capture branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. P-2 Issues/test failures with a fix SLA of 3 months T-cdc

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Dec 19, 2024

sql/importer.TestRandomParquetExports failed on release-24.3 @ 112484d48f818e6e8585fa944a150d06e5acbbfb:

=== RUN   TestRandomParquetExports
    test_log_scope.go:165: test logs captured to: outputs.zip/logTestRandomParquetExports1839873803
    test_log_scope.go:76: use -show-logs to present logs inline
    test_server_shim.go:152: automatically injected an external process virtual cluster under test; see comment at top of test_server_shim.go for details.
    exportparquet_test.go:252: exporting as parquet from random table with schema: 
         [[table3 CREATE TABLE public.table3 (
        	col3_0 INT8 NOT NULL,
        	col3_1 INT8 NOT NULL,
        	col3_2 BIT(0)[] NOT NULL,
        	col3_3 BPCHAR NOT NULL,
        	col3_4 DECIMAL NOT NULL,
        	col3_6 DECIMAL NOT NULL,
        	col3_7 INT8 NOT NULL AS (abs(col3_1)) VIRTUAL,
        	CONSTRAINT table3_pkey PRIMARY KEY (col3_6 DESC, col3_2 DESC, col3_3 ASC),
        	FAMILY fam_0_col3_0_col3_1 (col3_0, col3_1),
        	FAMILY fam_1_col3_2 (col3_2),
        	FAMILY fam_2_col3_4 (col3_4),
        	FAMILY fam_4_col3_6_col3_3 (col3_6, col3_3)
        )]]
    exportparquet_test.go:168: 
        	Error Trace:	pkg/sql/importer/exportparquet_test.go:168
        	            				pkg/sql/importer/exportparquet_test.go:134
        	            				pkg/sql/importer/exportparquet_test.go:121
        	            				pkg/sql/importer/exportparquet_test.go:262
        	Error:      	Not equal: 
        	            	expected: &tree.DBitArray{BitArray:bitarray.BitArray{words:[]uint64{}, lastBitsUsed:0x0}}
        	            	actual  : &tree.DBitArray{BitArray:bitarray.BitArray{words:[]uint64(nil), lastBitsUsed:0x0}}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -2,4 +2,3 @@
        	            	  BitArray: (bitarray.BitArray) {
        	            	-  words: ([]uint64) {
        	            	-  },
        	            	+  words: ([]uint64) <nil>,
        	            	   lastBitsUsed: (uint8) 0
        	Test:       	TestRandomParquetExports
    panic.go:626: -- test log scope end --
test logs left over in: outputs.zip/logTestRandomParquetExports1839873803
--- FAIL: TestRandomParquetExports (15.81s)

Parameters:

  • attempt=1
  • deadlock=true
  • run=2
  • shard=1
Help

See also: How To Investigate a Go Test Failure (internal)

Same failure on other branches

This test on roachdash | Improve this report!

Jira issue: CRDB-45732

@cockroach-teamcity cockroach-teamcity added branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-sql-queries SQL Queries Team labels Dec 19, 2024
@github-project-automation github-project-automation bot moved this to Triage in SQL Queries Dec 19, 2024
@mgartner
Copy link
Collaborator

@aerfrei I think this might also be related to #136718, can you take a look?

@mgartner mgartner added T-cdc and removed T-sql-queries SQL Queries Team labels Dec 26, 2024
Copy link

blathers-crl bot commented Dec 26, 2024

cc @cockroachdb/cdc

@blathers-crl blathers-crl bot added the A-cdc Change Data Capture label Dec 26, 2024
@mgartner mgartner removed this from SQL Queries Dec 26, 2024
@aerfrei
Copy link
Contributor

aerfrei commented Jan 2, 2025

Hi @mgartner! I noticed that this issue is for release 24.3. That change should only be in 25.1 (it wasn't backported) so I suspect this is unrelated. It also seems like the logs have been garbage collected so I couldn't find much more info. It did not reproduce locally when I stress tested it

@aerfrei
Copy link
Contributor

aerfrei commented Jan 3, 2025

I ran this test under stress another couple times on 24.3 and 24.2. I did end up getting a failure on 24.2 with this seed: -2051594066630352189. That's hopefully helpful.

Confirmed that this is happening even without my changes from this PR

@asg0451 asg0451 assigned rharding6373 and unassigned aerfrei Jan 6, 2025
@exalate-issue-sync exalate-issue-sync bot added the P-2 Issues/test failures with a fix SLA of 3 months label Jan 7, 2025
@rharding6373
Copy link
Collaborator

rharding6373 commented Jan 8, 2025

I've been taking a look since although the cause does not look likely to be because the the PR, CDC team does own EXPORT despite this test being owned by SQL Queries. The test fails in cases like the following (thanks @aerfrei for the seed, it was really helpful!):

CREATE TABLE t (b BIT(0));
INSERT INTO t VALUES (B'');
EXPORT INTO PARQUET 'nodelocal://1/outputfile' FROM SELECT * FROM t;

However, when I use pqrs cat on the outputfile on master and v24.2.7, I see the expected empty value: {b: []}, though I find it difficult to distinguish what's parquet reader behavior when it comes to empty fields/null. (Note: can't create tables with b BIT(0) on v24.2.0 -- syntax error: length for type bit must be at least 1). I initially thought there might be an issue with how we decode parquet in the testutils, but then I looked closer into this bit(0) error and it doesn't look like it's allowed in postgres either ("There are two SQL bit types: bit(n) and bit varying(n), where n is a positive integer"). So the real problem might be that there's a regression that allows bit(0) and the random table generator picked it up.

Edited to add repro for future reference (on a 24.3 branch):

./dev test pkg/sql/importer -f=TestRandomParquetExports -v --show-logs -- --test_env=COCKROACH_RANDOM_SEED=-2051594066630352189 > /tmp/out

@rharding6373
Copy link
Collaborator

I reproduced the BIT(0) create table error on v24.2.5, and it succeeds on v24.2.6.

@rharding6373
Copy link
Collaborator

Apparently this is intentional: #132944

So I think the conclusion is that the testutils parquet decoder can't handle this new type very well. I'm going to remove the release blocker.

@rharding6373 rharding6373 removed the release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. label Jan 8, 2025
@rharding6373 rharding6373 changed the title sql/importer: TestRandomParquetExports failed parquet: decoder should handle empty bit(0) arrays Jan 8, 2025
@mgartner
Copy link
Collaborator

mgartner commented Jan 8, 2025

Great job tracking this down @rharding6373! I didn't realize the new type would break this test - sorry about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cdc Change Data Capture branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. P-2 Issues/test failures with a fix SLA of 3 months T-cdc
Projects
None yet
Development

No branches or pull requests

4 participants