fix: Fix incorrect null results when unrelated child fields are missing from the requested type in readers

[zhztheplayer]

When reading data, either in dwrf or parquet, assuming we have data written with the following schema:

auto dataColumnType = ROW({"id", "amount"}, {BIGINT(), BIGINT()});
auto dataRowType = ROW({"order"}, {dataColumnType});

Meanwhile, we request the data with the following schema, which has a non-existing date: TIMESTAMP included:

auto requestedColumnType = ROW({"id", "date", "amount"}, {BIGINT(), TIMESTAMP(), BIGINT()});
auto requestedRowType = ROW({"order"}, {requestedColumnType});

Then, we request the following subfields by adding them to the column handle of order:

std::vector<common::Subfield> requiredSubfields;
requiredSubfields.emplace_back("order.id"); // Requires order.id
requiredSubfields.emplace_back("order.amount"); // Requires order.amount
connector::ColumnHandleMap assignments;
assignments["e"] = std::make_shared<HiveColumnHandle>(
  "order",
  HiveColumnHandle::ColumnType::kRegular,
  requestedColumnType,
  requestedColumnType,
  std::move(requiredSubfields));

Before the fix, Velox's answer is (wrong nulls for order.amount):

order.id	order.date	order.amount
1	null	null
2	null	null
3	null	null

After the fix, Velox's answer is (correct):

order.id	order.date	order.amount
1	null	100
2	null	150
3	null	200

[netlify]

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	78c70de
🔍 Latest deploy log	https://app.netlify.com/projects/meta-velox/deploys/68a35bab4c54be0008640631

[zhztheplayer]

@Yuhta @rui-mo this small fix is independent to #5962, but will allow me to get rid of a similar issue in the new Gluten delta reader in advance.

[Yuhta]

@zhztheplayer Adding a subfield in the middle of struct is not allowed, the new non-existing subfield must be appended to the end of the struct

[zhztheplayer]

the new non-existing subfield must be appended to the end of the struct

@Yuhta Shall we consider this as Presto's principle? Because Spark doesn't restrict on positions of the non-existing fields. We can have more tests from Velox side to guard Spark's use cases if needed.

[Yuhta]

@zhztheplayer This is not a compute engine property, it's a data warehouse property. Currently Velox only supports this positional schema evolution model; to support the name based one, it's a lot of work and testing to make sure both cases works. I would suggest starting by enhance table evolution fuzzer to add the name based mode, and see where are the gaps.

[zhztheplayer]

@zhztheplayer This is not a compute engine property, it's a data warehouse property. Currently Velox only supports this positional schema evolution model; to support the name based one, it's a lot of work and testing to make sure both cases works. I would suggest starting by enhance table evolution fuzzer to add the name based mode, and see where are the gaps.

Thanks and understood. I'll revisit the suggestion soon.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. If you'd still like this PR merged, please comment on the PR, make sure you've addressed reviewer comments, and rebase on the latest main. Thank you for your contributions!