Skip to content

Commit a849a59

Browse files
committed
[core] Remap predicate field index to projected read schema in InternalReadContext
Predicates are constructed against the latest table schema, so each LeafPredicate carries a field index that points to the column's position in the full table schema. When the query projects a subset of columns the read_schema built inside InternalReadContext::Create lays those columns out at different positions; the leaf's field index no longer matches the column it names. The strict field-id validation that runs immediately afterwards then fails: Paimon TableRead::Create error: Invalid: field obs_index has field idx 0 in input schema, mismatch field idx 1 in predicate The downstream LeafPredicateImpl::Test paths use field_index_ directly to index into arrow arrays / internal rows built from read_schema, so even if validation were relaxed, leaving the mismatch in place would silently read the wrong column. Walk the predicate tree once at context construction and rebuild each LeafPredicate with the field index resolved from the read_schema by field name. CompoundPredicate is reconstructed only when any descendant actually changed, otherwise the original shared_ptr is reused. GetPredicate() now returns the remapped predicate; the original (with latest-schema indices) is still available via the wrapped ReadContext if ever needed. CreateWithSchema lays the context over a different read_schema (e.g. the minimal column set for COUNT(*)), so it also remaps from the original FE predicate against the new read_schema rather than copying the already-remapped one whose indices are aligned with the original read_schema. Repro: -- DE table with btree global index on obs_index CREATE TABLE t ( clip_id STRING, obs_index INT, time_offset_ms BIGINT, collected_date DATE ) PARTITIONED BY (collected_date, clip_id) TBLPROPERTIES ( 'data-evolution.enabled' = 'true', 'global-index.btree.index-column' = 'obs_index', 'bucket' = '-1' ); -- after INSERT + CALL paimon.sys.create_global_index(...) SELECT clip_id, obs_index, time_offset_ms FROM t WHERE collected_date = '2026-05-26' AND clip_id = 'clip_a' AND obs_index BETWEEN 0 AND 10; -- before: TableRead::Create error (field idx mismatch) -- after: returns matching rows Coverage: InternalReadContext gains four cases — TestPredicateFieldIdxRemappedWhenProjected, TestPredicateUnchangedWhenAligned, TestCompoundPredicateRemap, TestPredicateOnFieldMissingFromReadSchema — covering the projected/aligned/compound/missing-field paths through the remap. Signed-off-by: duanyyyyyyy <yan.duan9759@gmail.com>
1 parent aac6240 commit a849a59

8 files changed

Lines changed: 204 additions & 32 deletions

File tree

include/paimon/predicate/leaf_predicate.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ enum class FieldType;
3232
/// Leaf node of a `Predicate` tree. Compares a field with literals.
3333
class PAIMON_EXPORT LeafPredicate : virtual public Predicate {
3434
public:
35+
/// The field's position in the schema this predicate is currently bound to.
36+
/// At construction the value reflects the schema the caller supplied to
37+
/// `PredicateBuilder`; predicates obtained from `InternalReadContext::GetPredicate()`
38+
/// have already been rebound to the projected `read_schema`. Use `FieldName()` as
39+
/// the schema-independent identity.
3540
int32_t FieldIndex() const {
3641
return field_index_;
3742
}

include/paimon/predicate/predicate_builder.h

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,25 @@ enum class FieldType;
3232
///
3333
/// PredicateBuilder provides static factory methods to create various types of predicates
3434
/// that can be used for filtering data in Paimon tables.
35+
///
36+
/// The `field_index` parameter accepted by every factory method is the position of the
37+
/// field in the schema the caller is working with — typically the latest table schema.
38+
/// `field_name` is the stable identity used to locate the field. When the resulting
39+
/// predicate is later attached to an `InternalReadContext`, `InternalReadContext::Create`
40+
/// automatically remaps `field_index` to the field's position in the projected
41+
/// `read_schema` using `field_name`, so callers do not need to track projection state
42+
/// themselves. `field_name` must therefore name a field present in the read schema.
3543
class PAIMON_EXPORT PredicateBuilder {
3644
public:
3745
PredicateBuilder() = delete;
3846
~PredicateBuilder() = delete;
3947

4048
/// Create an equality predicate (field == literal).
4149
///
42-
/// @param field_index The index of the field in read schema (0-based).
43-
/// @param field_name The name of the field.
50+
/// @param field_index The position of the field in the schema the caller is working
51+
/// with (0-based); rebound to the read schema by
52+
/// `InternalReadContext::Create`. See class doc for details.
53+
/// @param field_name The name of the field. Must exist in the read schema.
4454
/// @param field_type The data type of the field.
4555
/// @param literal The literal value to compare against.
4656
/// @return A shared pointer to the created Predicate object.
@@ -99,8 +109,10 @@ class PAIMON_EXPORT PredicateBuilder {
99109
///
100110
/// Tests whether the field value falls within the specified range (inclusive on both ends).
101111
///
102-
/// @param field_index The index of the field in read schema (0-based).
103-
/// @param field_name The name of the field.
112+
/// @param field_index The position of the field in the schema the caller is working
113+
/// with (0-based); rebound to the read schema by
114+
/// `InternalReadContext::Create`. See class doc for details.
115+
/// @param field_name The name of the field. Must exist in the read schema.
104116
/// @param field_type The data type of the field.
105117
/// @param included_lower_bound The lower bound of the range (inclusive).
106118
/// @param included_upper_bound The upper bound of the range (inclusive).

include/paimon/read_context.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,11 @@ class PAIMON_EXPORT ReadContextBuilder {
206206
/// It can significantly improve performance by reducing the amount of data
207207
/// that needs to be read and processed.
208208
///
209+
/// The caller is free to construct the predicate against any schema that contains the
210+
/// referenced fields (typically the latest table schema). `InternalReadContext::Create`
211+
/// rebinds each leaf's `field_index` to the field's position in the projected read
212+
/// schema using `field_name`, so the predicate does not need to be projection-aware.
213+
///
209214
/// @param predicate Shared pointer to the predicate for data filtering.
210215
/// @return Reference to this builder for method chaining.
211216
ReadContextBuilder& SetPredicate(const std::shared_ptr<Predicate>& predicate);

src/paimon/common/predicate/predicate_builder.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@
4242
namespace paimon {
4343
enum class FieldType;
4444

45-
// TODO(xinyu.lxy): predicate field_index use index in read schema now, but java paimon use index
46-
// in file schema
4745
std::shared_ptr<Predicate> PredicateBuilder::Equal(int32_t field_index,
4846
const std::string& field_name,
4947
const FieldType& field_type,

src/paimon/core/operation/internal_read_context.cpp

Lines changed: 72 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
#include <utility>
2020

21+
#include "paimon/common/predicate/compound_predicate_impl.h"
22+
#include "paimon/common/predicate/leaf_predicate_impl.h"
2123
#include "paimon/common/predicate/predicate_validator.h"
2224
#include "paimon/common/table/special_fields.h"
2325
#include "paimon/common/types/data_field.h"
@@ -29,6 +31,50 @@ class Schema;
2931
} // namespace arrow
3032

3133
namespace paimon {
34+
namespace {
35+
// Remap each leaf predicate's field index to the position of its field in
36+
// `read_schema`. The field name is the stable identity used for lookup. The
37+
// upstream (Java SDK / FE) constructs predicates against the latest table
38+
// schema, so when a query projects a subset of columns the predicate's field
39+
// index no longer matches the position in the projected `read_schema`. The
40+
// downstream predicate-test code uses the field index directly, so leaving
41+
// the mismatch in place would silently read the wrong column. Remap once at
42+
// context construction so every subsequent reader sees a predicate already
43+
// aligned with `read_schema`.
44+
Result<std::shared_ptr<Predicate>> RemapPredicateFieldIndex(
45+
const arrow::Schema& read_schema, const std::shared_ptr<Predicate>& predicate) {
46+
if (auto leaf = std::dynamic_pointer_cast<LeafPredicateImpl>(predicate)) {
47+
int32_t new_index = read_schema.GetFieldIndex(leaf->FieldName());
48+
if (new_index == -1) {
49+
return Status::Invalid(
50+
fmt::format("field {} does not exist in schema", leaf->FieldName()));
51+
}
52+
if (new_index == leaf->FieldIndex()) {
53+
return predicate;
54+
}
55+
return std::static_pointer_cast<Predicate>(leaf->NewLeafPredicate(new_index));
56+
}
57+
if (auto compound = std::dynamic_pointer_cast<CompoundPredicateImpl>(predicate)) {
58+
std::vector<std::shared_ptr<Predicate>> remapped_children;
59+
remapped_children.reserve(compound->Children().size());
60+
bool any_changed = false;
61+
for (const auto& child : compound->Children()) {
62+
PAIMON_ASSIGN_OR_RAISE(auto remapped, RemapPredicateFieldIndex(read_schema, child));
63+
if (remapped != child) {
64+
any_changed = true;
65+
}
66+
remapped_children.push_back(std::move(remapped));
67+
}
68+
if (!any_changed) {
69+
return predicate;
70+
}
71+
return std::static_pointer_cast<Predicate>(
72+
compound->NewCompoundPredicate(remapped_children));
73+
}
74+
return predicate;
75+
}
76+
} // namespace
77+
3278
Result<std::unique_ptr<InternalReadContext>> InternalReadContext::Create(
3379
const std::shared_ptr<ReadContext>& context, const std::shared_ptr<TableSchema>& table_schema,
3480
const std::map<std::string, std::string>& options) {
@@ -95,34 +141,48 @@ Result<std::unique_ptr<InternalReadContext>> InternalReadContext::Create(
95141
auto read_schema = DataField::ConvertDataFieldsToArrowSchema(read_data_fields);
96142
// validate read schema to avoid redundant fields
97143
PAIMON_RETURN_NOT_OK(ArrowSchemaValidator::ValidateSchemaWithFieldId(*read_schema));
98-
// validate predicate
144+
// remap and validate predicate
145+
std::shared_ptr<Predicate> remapped_predicate;
99146
if (context->GetPredicate()) {
147+
PAIMON_ASSIGN_OR_RAISE(remapped_predicate,
148+
RemapPredicateFieldIndex(*read_schema, context->GetPredicate()));
100149
PAIMON_RETURN_NOT_OK(PredicateValidator::ValidatePredicateWithSchema(
101-
*read_schema, context->GetPredicate(), /*validate_field_idx=*/true));
102-
PAIMON_RETURN_NOT_OK(
103-
PredicateValidator::ValidatePredicateWithLiterals(context->GetPredicate()));
150+
*read_schema, remapped_predicate, /*validate_field_idx=*/true));
151+
PAIMON_RETURN_NOT_OK(PredicateValidator::ValidatePredicateWithLiterals(remapped_predicate));
104152
}
105153

106-
return std::unique_ptr<InternalReadContext>(
107-
new InternalReadContext(context, table_schema, read_schema, core_options));
154+
return std::unique_ptr<InternalReadContext>(new InternalReadContext(
155+
context, table_schema, read_schema, core_options, std::move(remapped_predicate)));
108156
}
109157

110158
InternalReadContext::InternalReadContext(const std::shared_ptr<ReadContext>& read_context,
111159
const std::shared_ptr<TableSchema>& table_schema,
112160
const std::shared_ptr<arrow::Schema>& read_schema,
113-
const CoreOptions& options)
161+
const CoreOptions& options,
162+
std::shared_ptr<Predicate> remapped_predicate)
114163
: read_context_(read_context),
115164
table_schema_(table_schema),
116165
read_schema_(read_schema),
117-
options_(options) {}
166+
options_(options),
167+
remapped_predicate_(std::move(remapped_predicate)) {}
118168

119169
Result<std::shared_ptr<InternalReadContext>> InternalReadContext::CreateWithSchema(
120170
const std::shared_ptr<InternalReadContext>& original,
121171
const std::shared_ptr<arrow::Schema>& new_read_schema) {
122-
// Create a new InternalReadContext sharing all properties except read_schema.
123-
// The new read_schema is the minimal column set for COUNT(*).
124-
return std::shared_ptr<InternalReadContext>(new InternalReadContext(
125-
original->read_context_, original->table_schema_, new_read_schema, original->options_));
172+
// Create a new InternalReadContext sharing all properties except read_schema. The
173+
// new read_schema is a different column layout (e.g. the minimal column set for
174+
// COUNT(*)), so the predicate's field index needs to be re-aligned against it.
175+
// Remap from the original (latest-schema-indexed) predicate so the mapping is
176+
// computed against new_read_schema directly.
177+
std::shared_ptr<Predicate> remapped_predicate;
178+
if (original->read_context_->GetPredicate()) {
179+
PAIMON_ASSIGN_OR_RAISE(
180+
remapped_predicate,
181+
RemapPredicateFieldIndex(*new_read_schema, original->read_context_->GetPredicate()));
182+
}
183+
return std::shared_ptr<InternalReadContext>(
184+
new InternalReadContext(original->read_context_, original->table_schema_, new_read_schema,
185+
original->options_, std::move(remapped_predicate)));
126186
}
127187

128188
} // namespace paimon

src/paimon/core/operation/internal_read_context.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,13 @@ class InternalReadContext {
6262
const std::vector<std::string>& GetPrimaryKeys() const {
6363
return table_schema_->PrimaryKeys();
6464
}
65+
// Returns the predicate with each leaf's field index already remapped to its position
66+
// in `read_schema_`. Upstream constructs predicates against the latest table schema,
67+
// so when the query projects a subset of columns the original indices no longer
68+
// match the projected schema. The remapping is done once at context construction; all
69+
// subsequent reader code can use this index directly.
6570
const std::shared_ptr<Predicate>& GetPredicate() const {
66-
return read_context_->GetPredicate();
71+
return remapped_predicate_;
6772
}
6873
bool EnablePredicateFilter() const {
6974
return read_context_->EnablePredicateFilter();
@@ -110,12 +115,13 @@ class InternalReadContext {
110115
InternalReadContext(const std::shared_ptr<ReadContext>& read_context,
111116
const std::shared_ptr<TableSchema>& table_schema,
112117
const std::shared_ptr<arrow::Schema>& read_schema,
113-
const CoreOptions& options);
118+
const CoreOptions& options, std::shared_ptr<Predicate> remapped_predicate);
114119

115120
std::shared_ptr<ReadContext> read_context_;
116121
std::shared_ptr<TableSchema> table_schema_;
117122
std::shared_ptr<arrow::Schema> read_schema_;
118123
CoreOptions options_;
124+
std::shared_ptr<Predicate> remapped_predicate_;
119125
};
120126

121127
} // namespace paimon

src/paimon/core/operation/internal_read_context_test.cpp

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525
#include "paimon/core/schema/schema_manager.h"
2626
#include "paimon/defs.h"
2727
#include "paimon/fs/local/local_file_system.h"
28+
#include "paimon/predicate/compound_predicate.h"
29+
#include "paimon/predicate/leaf_predicate.h"
30+
#include "paimon/predicate/predicate_builder.h"
2831
#include "paimon/status.h"
2932
#include "paimon/testing/utils/testharness.h"
3033

@@ -191,4 +194,99 @@ TEST(InternalReadContext, TestReadWithFieldIdsAndSpecialFields) {
191194
}
192195
}
193196

197+
// Upstream predicates are constructed against the latest table schema. When the query
198+
// projects a subset of columns, the read schema built inside InternalReadContext::Create
199+
// lays those columns out at different positions and the original field_index no longer
200+
// matches. The remapping path in Create() must rewrite each leaf predicate so its
201+
// field_index points to the column's position in the projected read schema.
202+
TEST(InternalReadContext, TestPredicateFieldIdxRemappedWhenProjected) {
203+
std::string path = paimon::test::GetDataDir() + "/orc/append_09.db/append_09";
204+
ReadContextBuilder context_builder(path);
205+
// read_schema lays out (f3, f0) — f3 ends up at position 0, f0 at position 1, while
206+
// the latest table schema has them at 3 and 0 respectively.
207+
context_builder.SetReadSchema({"f3", "f0"});
208+
// Predicate was constructed against the latest schema, so f3 carries field_index 3.
209+
auto predicate =
210+
PredicateBuilder::Equal(/*field_index=*/3, "f3", FieldType::DOUBLE, Literal(1.5));
211+
context_builder.SetPredicate(predicate);
212+
ASSERT_OK_AND_ASSIGN(auto read_context, context_builder.Finish());
213+
SchemaManager schema_manager(std::make_shared<LocalFileSystem>(), read_context->GetPath());
214+
ASSERT_OK_AND_ASSIGN(auto table_schema, schema_manager.ReadSchema(0));
215+
ASSERT_OK_AND_ASSIGN(auto internal_context,
216+
InternalReadContext::Create(std::move(read_context), table_schema,
217+
table_schema->Options()));
218+
auto leaf = std::dynamic_pointer_cast<LeafPredicate>(internal_context->GetPredicate());
219+
ASSERT_NE(leaf, nullptr);
220+
EXPECT_EQ(leaf->FieldName(), "f3");
221+
// f3 is the first column in the projected read schema.
222+
EXPECT_EQ(leaf->FieldIndex(), 0);
223+
}
224+
225+
// When the predicate already aligns with the read schema, remapping should be a no-op
226+
// and return the original shared_ptr without reconstructing the leaf.
227+
TEST(InternalReadContext, TestPredicateUnchangedWhenAligned) {
228+
std::string path = paimon::test::GetDataDir() + "/orc/append_09.db/append_09";
229+
ReadContextBuilder context_builder(path);
230+
// No projection -> read schema matches the latest schema: f0(0), f1(1), f2(2), f3(3).
231+
auto predicate =
232+
PredicateBuilder::Equal(/*field_index=*/3, "f3", FieldType::DOUBLE, Literal(1.5));
233+
context_builder.SetPredicate(predicate);
234+
ASSERT_OK_AND_ASSIGN(auto read_context, context_builder.Finish());
235+
SchemaManager schema_manager(std::make_shared<LocalFileSystem>(), read_context->GetPath());
236+
ASSERT_OK_AND_ASSIGN(auto table_schema, schema_manager.ReadSchema(0));
237+
ASSERT_OK_AND_ASSIGN(auto internal_context,
238+
InternalReadContext::Create(std::move(read_context), table_schema,
239+
table_schema->Options()));
240+
// shared_ptr equality: remap returned the input unchanged.
241+
EXPECT_EQ(predicate.get(), internal_context->GetPredicate().get());
242+
}
243+
244+
// CompoundPredicate is recursively remapped: every nested leaf must point to the
245+
// column's position in the projected read schema.
246+
TEST(InternalReadContext, TestCompoundPredicateRemap) {
247+
std::string path = paimon::test::GetDataDir() + "/orc/append_09.db/append_09";
248+
ReadContextBuilder context_builder(path);
249+
context_builder.SetReadSchema({"f3", "f0"});
250+
// AND(f3 == 1.5, f0 == "x") with field indices from the latest schema.
251+
auto left = PredicateBuilder::Equal(/*field_index=*/3, "f3", FieldType::DOUBLE, Literal(1.5));
252+
auto right = PredicateBuilder::Equal(/*field_index=*/0, "f0", FieldType::STRING,
253+
Literal(FieldType::STRING, "x", 1));
254+
ASSERT_OK_AND_ASSIGN(auto compound, PredicateBuilder::And({left, right}));
255+
context_builder.SetPredicate(compound);
256+
ASSERT_OK_AND_ASSIGN(auto read_context, context_builder.Finish());
257+
SchemaManager schema_manager(std::make_shared<LocalFileSystem>(), read_context->GetPath());
258+
ASSERT_OK_AND_ASSIGN(auto table_schema, schema_manager.ReadSchema(0));
259+
ASSERT_OK_AND_ASSIGN(auto internal_context,
260+
InternalReadContext::Create(std::move(read_context), table_schema,
261+
table_schema->Options()));
262+
auto remapped_compound =
263+
std::dynamic_pointer_cast<CompoundPredicate>(internal_context->GetPredicate());
264+
ASSERT_NE(remapped_compound, nullptr);
265+
ASSERT_EQ(remapped_compound->Children().size(), 2);
266+
auto remapped_left = std::dynamic_pointer_cast<LeafPredicate>(remapped_compound->Children()[0]);
267+
auto remapped_right =
268+
std::dynamic_pointer_cast<LeafPredicate>(remapped_compound->Children()[1]);
269+
ASSERT_NE(remapped_left, nullptr);
270+
ASSERT_NE(remapped_right, nullptr);
271+
EXPECT_EQ(remapped_left->FieldName(), "f3");
272+
EXPECT_EQ(remapped_left->FieldIndex(), 0);
273+
EXPECT_EQ(remapped_right->FieldName(), "f0");
274+
EXPECT_EQ(remapped_right->FieldIndex(), 1);
275+
}
276+
277+
// Field not present in the projected read schema must surface as an Invalid status.
278+
TEST(InternalReadContext, TestPredicateOnFieldMissingFromReadSchema) {
279+
std::string path = paimon::test::GetDataDir() + "/orc/append_09.db/append_09";
280+
ReadContextBuilder context_builder(path);
281+
context_builder.SetReadSchema({"f3", "f0"});
282+
auto predicate = PredicateBuilder::Equal(/*field_index=*/1, "f1", FieldType::INT, Literal(7));
283+
context_builder.SetPredicate(predicate);
284+
ASSERT_OK_AND_ASSIGN(auto read_context, context_builder.Finish());
285+
SchemaManager schema_manager(std::make_shared<LocalFileSystem>(), read_context->GetPath());
286+
ASSERT_OK_AND_ASSIGN(auto table_schema, schema_manager.ReadSchema(0));
287+
ASSERT_NOK_WITH_MSG(
288+
InternalReadContext::Create(std::move(read_context), table_schema, table_schema->Options()),
289+
"field f1 does not exist in schema");
290+
}
291+
194292
} // namespace paimon::test

src/paimon/core/table/source/table_read_test.cpp

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,18 +67,6 @@ TEST(TableReadTest, TestReadWithInvalidContext) {
6767
ASSERT_NOK_WITH_MSG(TableRead::Create(std::move(read_context)),
6868
"schema type double mismatches predicate field type BIGINT");
6969
}
70-
{
71-
// field idx in predicate mismatch in schema
72-
auto predicate = PredicateBuilder::Equal(/*field_index=*/2, /*field_name=*/"f3",
73-
FieldType::DOUBLE, Literal(15.0));
74-
ReadContextBuilder context_builder(path);
75-
context_builder.SetReadSchema({"f3", "f0", "f1"});
76-
context_builder.SetPredicate(predicate);
77-
ASSERT_OK_AND_ASSIGN(auto read_context, context_builder.Finish());
78-
ASSERT_NOK_WITH_MSG(
79-
TableRead::Create(std::move(read_context)),
80-
"field f3 has field idx 0 in input schema, mismatch field idx 2 in predicate");
81-
}
8270
{
8371
// literal cannot be null
8472
auto predicate = PredicateBuilder::Equal(/*field_index=*/3, /*field_name=*/"f3",

0 commit comments

Comments
 (0)