Skip to content

Conversation

@parthchandra
Copy link
Contributor

Changes in CometScanExec to support the full native reader caused the original reader. This fixes the issues so that all Comet unit tests pass with the original native scan. The only tests that do not pass are the plan stability tests where the 'expected' plans expect CometNativeScan but if full native is not turned on, the plan produces a CometScan (which is expected).

@andygrove andygrove changed the title fix: fix regressions original comet native scal implementation fix: [comet-parquet-exec] fix regressions original comet native scal implementation Dec 13, 2024
@andygrove andygrove merged commit 8563edf into apache:comet-parquet-exec Dec 13, 2024
16 of 72 checks passed
Preconditions.checkState(
t.isPrimitive() && !t.isRepetition(Type.Repetition.REPEATED),
"Complex type is not supported");
// Preconditions.checkState(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is for the second implementation. Will remove this later

true
case t: DataType if t.typeName == "timestamp_ntz" => true
case _: StructType => true
case _: StructType
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With only the original scan enabled, this caused CometScan to be used for Struct types (which it did not support).


override lazy val (outputPartitioning, outputOrdering): (Partitioning, Seq[SortOrder]) =
(wrapped.outputPartitioning, wrapped.outputOrdering)
override lazy val (outputPartitioning, outputOrdering): (Partitioning, Seq[SortOrder]) = {
Copy link
Contributor Author

@parthchandra parthchandra Dec 13, 2024

Choose a reason for hiding this comment

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

@viirya - The previous fix to address outputPartitioning (using inputRDD) was not correct and caused multiple test failures. This is a different attempt (and at least all the tests pass).
If this is a bucketedScan, we fall back to the wrapped FileSourceScanLike implementation but for non bucketed case since FileSourceScanLike always returned 0 partitions, we override the behaviour, setting the num of partitions to the number of files.
I'm not entirely sure this covers all cases so please advise.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants