Skip to content

Data, Spark: Add format model TCK coverage for vectorized reads (WIP) - #17610

Draft
joyhaldar wants to merge 1 commit into
apache:mainfrom
joyhaldar:spark-vectorized-formatmodel-tck
Draft

Data, Spark: Add format model TCK coverage for vectorized reads (WIP)#17610
joyhaldar wants to merge 1 commit into
apache:mainfrom
joyhaldar:spark-vectorized-formatmodel-tck

Conversation

@joyhaldar

Copy link
Copy Markdown
Contributor

Adds a TCK subclass for Spark's vectorized ColumnarBatch read path.

Four base class changes were needed:

  • readOnly() - vectorized models register readers only, so write tests are skipped.
  • supportedFormats() - Avro has no ColumnarBatch model registered
  • assertRecordsEqual() - read tests built expectations via convertToEngineRecords, which can't work for a batch type
  • Read assertions moved inside try-with-resources, columnar batches don't survive the reader closing

Co-authored-by: Joy Haldar <joy.haldar@target.com>
@joyhaldar joyhaldar changed the title [WIP] Data, Spark: Add format model TCK coverage for vectorized reads Data, Spark: Add format model TCK coverage for vectorized reads (WIP) Aug 11, 2026
return false;
}

protected Set<FileFormat> supportedFormats() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't really like this one.

Could we just re-define FILE_FORMATS, and FORMAT_AND_GENERATOR in TestSparkVectorizedFormatModel?

  private static final FileFormat[] FILE_FORMATS = {FileFormat.ORC, FileFormat.PARQUET};
  private static final List<Arguments> FORMAT_AND_GENERATOR

This relies on the fact, how junit works internally, but no change is required in all of the tests.

I'm open for other ideas as well, this is just the first one occured to me

return false;
}

protected boolean readOnly() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shall we split the base test class to read-tests, and read+write-tests?
Using this method in all test seem awkward a bit.

If we decide so, we might want to do it in a different PR

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants