Skip to content

DeltaBitPackEncoderConversion: Fix panic message on invalid type#9552

Merged
alamb merged 7 commits intoapache:mainfrom
progval:DeltaBitPackEncoderConversion-error-msg
Mar 17, 2026
Merged

DeltaBitPackEncoderConversion: Fix panic message on invalid type#9552
alamb merged 7 commits intoapache:mainfrom
progval:DeltaBitPackEncoderConversion-error-msg

Conversation

@progval
Copy link
Contributor

@progval progval commented Mar 13, 2026

Which issue does this PR close?

Rationale for this change

DeltaBitPackDecoder supports Int32Type, UInt32Type, Int64Type, and UInt64Type; but the error message claimed it supported only Int32Type and Int64Type

What changes are included in this PR?

  • changed the error message
  • deduplicated the string
  • extended ensure_phys_ty!() to allow anything panic!() does

Are these changes tested?

no

Are there any user-facing changes?

only the panic message

@github-actions github-actions bot added the parquet Changes to the parquet crate label Mar 13, 2026
@progval progval force-pushed the DeltaBitPackEncoderConversion-error-msg branch from 30a208a to b2765c3 Compare March 13, 2026 17:32
@progval progval force-pushed the DeltaBitPackEncoderConversion-error-msg branch from b2765c3 to 6d517de Compare March 13, 2026 17:42
@Dandandan
Copy link
Contributor

@progval Can you add a test for this?

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me -- thank you @progval for the improvement

I notice there is a CI test that is failing, which I will fix

@@ -0,0 +1,50 @@
// Licensed to the Apache Software Foundation (ASF) under one
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it is strange to have a new integration test just for this error message, I agree starting to have an arrow_writer suite is a good step forward

@alamb alamb merged commit d1ec770 into apache:main Mar 17, 2026
17 checks passed
@alamb
Copy link
Contributor

alamb commented Mar 17, 2026

Thanks again @progval

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parquet Changes to the parquet crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"DeltaBitPackDecoder only supports Int32Type and Int64Type" but unsigned types are supported too

3 participants