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

incremental query file and partition pruning #2

Merged

Conversation

psendyk
Copy link
Collaborator

@psendyk psendyk commented Sep 15, 2023

Makes the MOR table incremental queries faster via following optimizations:

  • when listing file status of the affected files, also filter on hoodie.datasource.read.incr.path.glob match
  • in case of an incremental pull, prune partition paths before listing instead of pruning data files after listing, this is done via new parameter hoodie.datasource.read.incr.partition.glob

@psendyk psendyk force-pushed the pawel/incremental-query-pruning branch 2 times, most recently from 0c11544 to 9c4dd0f Compare September 15, 2023 22:07
@psendyk psendyk force-pushed the pawel/incremental-query-pruning branch from 9c4dd0f to 99c76dc Compare September 15, 2023 22:13
@@ -102,13 +102,14 @@ case class MergeOnReadIncrementalRelation(override val sqlContext: SQLContext,
val fsView = new HoodieTableFileSystemView(metaClient, timeline, affectedFilesInCommits)

val modifiedPartitions = getWritePartitionPaths(commitsMetadata)
val globMatcher = new GlobPattern("*" + globPartitionPattern)

Choose a reason for hiding this comment

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

Might auto-prefixing the glob with an * result in potentially weird behavior?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What do you mean by weird? Honestly didn't think about it much, just kept it consistent with the existing logic at

.

@psendyk psendyk merged commit 5316b61 into heap:release-0.13.0 Sep 18, 2023
psendyk added a commit that referenced this pull request Apr 10, 2024
psendyk pushed a commit that referenced this pull request Apr 10, 2024
…ernalWriterHelper::write(...) (apache#10272)

Issue:
There are two configs which when set in a certain manner throws exceptions or asserts
1. Configs to disable populating metadata fields (for each row)
2. Configs to drop partition columns (to save storage space) from a row

With #1 and #2, partition paths cannot be deduced using partition columns (as the partition columns are dropped higher up the stack.
BulkInsertDataInternalWriterHelper::write(...) relied on metadata fields to extract partition path in such cases.
But with #1 it is not possible resulting in asserts/exceptions.

The fix is to push down the dropping of partition columns down the stack after partition path is computed.
The fix manipulates the raw 'InternalRow' row structure by only copying the relevent fields into a new 'InternalRow' structure.
Each row is processed individually to drop the partition columns and copy it a to new 'InternalRow'

Co-authored-by: Vinaykumar Bhat <vinay@onehouse.ai>
psendyk pushed a commit that referenced this pull request Apr 10, 2024
There are a couple of issues in how functional indexes are managed.
1. HoodieSparkFunctionalIndexClient::create(...) was failing a register a functional index iff a (different) functional
index was already created. Fixed this check by looking up the index-name in the FunctionalIndexMetadata
2. HoodieTableConfig `TABLE_METADATA_PARTITIONS` and `TABLE_METADATA_PARTITIONS_INFLIGHT` should actually store the Metadata
partition path. While the path is contained in the `MeatadatPartitionType` for most of the indexes, it is not correct for
functional-index. MeatadatPartitionType.FUNCTIONAL_INDEX only stores the prefix (i.e func_index_). The actual partition
path needs to be extracted from the index-name.
3. Because of #2, most of the helper methods that operate on metadata-partitions, should take partition-path (and not partition-type)

This PR addresses the problem listed above. This fix is required to add SQL support for secondary-indexes (the configs
for which will be based on functional-index-config).

Note that there are still issues with some functional-index operations (like drop index / delete partition)
because of the issues listed here. Those will be fixed in a subsequent PR.

Co-authored-by: Vinaykumar Bhat <vinay@onehouse.ai>
psendyk pushed a commit that referenced this pull request Apr 10, 2024
…ernalWriterHelper::write(...) (apache#10272)

Issue:
There are two configs which when set in a certain manner throws exceptions or asserts
1. Configs to disable populating metadata fields (for each row)
2. Configs to drop partition columns (to save storage space) from a row

With #1 and #2, partition paths cannot be deduced using partition columns (as the partition columns are dropped higher up the stack.
BulkInsertDataInternalWriterHelper::write(...) relied on metadata fields to extract partition path in such cases.
But with #1 it is not possible resulting in asserts/exceptions.

The fix is to push down the dropping of partition columns down the stack after partition path is computed.
The fix manipulates the raw 'InternalRow' row structure by only copying the relevent fields into a new 'InternalRow' structure.
Each row is processed individually to drop the partition columns and copy it a to new 'InternalRow'

Co-authored-by: Vinaykumar Bhat <vinay@onehouse.ai>
psendyk added a commit that referenced this pull request Apr 11, 2024
psendyk added a commit that referenced this pull request Apr 12, 2024
ramsgoli pushed a commit that referenced this pull request Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants