Skip to content

Conversation

@yuzelin
Copy link
Contributor

@yuzelin yuzelin commented Jul 1, 2025

Purpose

Reproduce steps:

CREATE TABLE T (a STRING);
INSERT INTO T VALUES ('9'), ('10'), ('11');
ALTER TABLE T MODIFY (a INT);
SELECT * FROM T WHERE a > 9; -- return empty but expected (10), (11)

The reason is stats type has been modified (similar to #4705 ).

Tests

Note:
SchemaEvolutionTest#testAddField for nullSafe arg.

API and Format

Documentation

@yuzelin yuzelin force-pushed the fix-stats-filter branch from 67a6f86 to 33677bd Compare July 1, 2025 12:32
@yuzelin yuzelin changed the title [core] Fix stats filter by pushdown after schema evolution (#4705) [core] Fix stats filter by pushdown after schema evolution Jul 1, 2025
@yuzelin
Copy link
Contributor Author

yuzelin commented Jul 1, 2025

@JingsongLi can we delete SchemaEvolutionTableTestBase and related tests? It depends on filter evolution even the type is changed a lot but this commit forbid this behavior.

@yuzelin yuzelin force-pushed the fix-stats-filter branch from 33677bd to b3ca28a Compare July 2, 2025 02:20
@yuzelin
Copy link
Contributor Author

yuzelin commented Jul 2, 2025

SchemaEvolutionTableTestBase and related tests are removed.

@yuzelin yuzelin force-pushed the fix-stats-filter branch from 4655d8a to 6e8bea3 Compare July 2, 2025 03:28
@JingsongLi
Copy link
Contributor

SchemaEvolutionTableTestBase and related tests are removed.

now which tests can cover the cases here?

@yuzelin
Copy link
Contributor Author

yuzelin commented Jul 4, 2025

SchemaEvolutionTableTestBase and related tests are removed.

now which tests can cover the cases here?

I will check all the tests and find tests that can be kept.

@yuzelin yuzelin force-pushed the fix-stats-filter branch from 6e8bea3 to d399ed9 Compare July 9, 2025 09:13
Collections.singletonList(filter),
isKeyStats,
nullSafe));
return devolved.isEmpty() ? null : devolved.get(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Here should consider and filter, should deal with multiple predicates situation.


// Stats filter is unsafe if it should be devolved and the devolving is unsafe
// null safe: it's safe if the predicate field is added later and filter it on the old schema
public boolean statsFilterUnsafe(ManifestEntry entry, Predicate statsFilter) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not return bool, we should return predicate, here should consider and filter, should deal with multiple predicates situation.

this.bucketSelectConverter = bucketSelectConverter;
this.fieldKeyStatsConverters =
new SimpleStatsEvolutions(
sid -> keyValueFieldsExtractor.keyFields(scanTableSchema(sid)),
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not introduce isKeyFilter. We should remove _KEY_ for keyFields here.

@JingsongLi
Copy link
Contributor

+1

@JingsongLi JingsongLi merged commit 2a70b56 into apache:master Jul 22, 2025
22 checks passed
@yuzelin yuzelin deleted the fix-stats-filter branch August 25, 2025 03:40
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