Skip to content

[GLUTEN-1433][VL][FOLLOWUP] Fix TimestampNTZ schema validation never matching due to Scala object simpleName suffix#12029

Merged
rui-mo merged 1 commit into
apache:mainfrom
liujiayi771:ntz-primitive-check
May 6, 2026
Merged

[GLUTEN-1433][VL][FOLLOWUP] Fix TimestampNTZ schema validation never matching due to Scala object simpleName suffix#12029
rui-mo merged 1 commit into
apache:mainfrom
liujiayi771:ntz-primitive-check

Conversation

@liujiayi771
Copy link
Copy Markdown
Contributor

@liujiayi771 liujiayi771 commented May 3, 2026

Summary

isPrimitiveType in VeloxValidatorApi uses dt.getClass.getSimpleName == "TimestampNTZType" to check for TimestampNTZ type. However, TimestampNTZType is a Scala case object, so getClass.getSimpleName returns "TimestampNTZType$" (with a trailing $), causing the check to always fail. This means validateSchema rejects TimestampNTZ even when enableTimestampNtzValidation is set to false.

Fix: Use dt.catalogString == "timestamp_ntz" instead, which is reliable regardless of how the type is defined in Scala.

Test: Added a test in VeloxParquetDataTypeValidationSuite to verify that validateSchema correctly accepts/rejects TimestampNTZ based on the enableTimestampNtzValidation config.

Related issue: #1433

…matching due to Scala object simpleName suffix

TimestampNTZType is a Scala case object, so getClass.getSimpleName
returns "TimestampNTZType$" (with trailing $), causing the equality
check against "TimestampNTZType" to always fail. This meant
validateSchema would reject TimestampNTZ even when
enableTimestampNtzValidation was set to false.

Use catalogString == "timestamp_ntz" instead, which is reliable
regardless of how the type is defined in Scala.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added the VELOX label May 3, 2026
@FelixYBW FelixYBW requested a review from rui-mo May 4, 2026 01:31
@liujiayi771
Copy link
Copy Markdown
Contributor Author

cc @rui-mo @Mariamalmesfer. Could you help to review?

Copy link
Copy Markdown
Contributor

@Mariamalmesfer Mariamalmesfer left a comment

Choose a reason for hiding this comment

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

LGTM
@rui-mo can you take a look?

Copy link
Copy Markdown
Contributor

@zml1206 zml1206 left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Copy Markdown
Contributor

@rui-mo rui-mo left a comment

Choose a reason for hiding this comment

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

Thanks @liujiayi771. This is a known issue, and I made a fix in our dev branch: https://github.com/apache/gluten/tree/ts_ntz_dev.

@rui-mo rui-mo merged commit 6d6a1af into apache:main May 6, 2026
103 of 104 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants