drin opened a new pull request, #18187:
URL: https://github.com/apache/datafusion/pull/18187
## Which issue does this PR close?
Closes #18122
## Rationale for this change
Existing behavior is to use the `relation` field of `ColumnRelation` message
to construct a `TableReference` ([mod.rs#L146][fromcol_proto],
[mod.rs#L171][tryfrom_proto]). However, the `relation` field
is a string and `From<String> for TableReference` always calls
parse_identifiers_normalized with `ignore_case: False`, which always
normalizes the identifier to lower case
([TableReference::parse_str][parse_str]).
For a description of the bug at a bit of a higher level, see #18122.
## What changes are included in this PR?
This PR introduces the following:
1. An implementation `From<protobuf::ColumnRelation>` and
`From<&protobuf::ColumnRelation>` for
`TableReference`.
2. Updated logic in `TryFrom<&protobuf::DFSchema>` for `DFSchema` and in
`From<protobuf::Column>` for `Column` that correctly leads to the new `From`
impls for `TableReference` to be invoked.
3. A new method, `TableReference::parse_str_normalized`, that parses an
identifier without normalizing it, with some logic from
`TableReference::parse_str` being refactored to accommodate code reuse.
## Are these changes tested?
Commit a355196 adds a new test case, `roundtrip_mixed_case_table_reference`,
that tests the desired behavior.
The existing behavior (without the fix in 0616df2 and with the extra line
`println!("{}", server_logical_plan.display_indent_schema());`):
```
cargo test "roundtrip_mixed_case_table_reference" --test proto_integration
-- --nocapture
Compiling datafusion-proto v48.0.1
(/Users/aldrinm/code/bauplanlabs/datafusion/octalene-datafusion/datafusion/proto)
Finished `test` profile [unoptimized + debuginfo] target(s) in 1.56s
Running tests/proto_integration.rs
(target/debug/deps/proto_integration-775454d70979734b)
running 1 test
thread 'cases::roundtrip_logical_plan::roundtrip_mixed_case_table_reference'
panicked at datafusion/proto/tests/cases/roundtrip_logical_plan.rs:2690:5:
assertion `left == right` failed
left: "Filter: TestData.a = Int64(1) [a:Int64;N]\n TableScan: TestData
projection=[a], partial_filters=[TestData.a = Int64(1)] [a:Int64;N]"
right: "Filter: testdata.a = Int64(1) [a:Int64;N]\n TableScan: TestData
projection=[a], partial_filters=[testdata.a = Int64(1)] [a:Int64;N]"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test cases::roundtrip_logical_plan::roundtrip_mixed_case_table_reference ...
FAILED
failures:
failures:
cases::roundtrip_logical_plan::roundtrip_mixed_case_table_reference
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 112 filtered
out; finished in 0.09s
```
With the fix implemented (0616df2):
```
running 1 test
Filter: TestData.a = Int64(1) [a:Int64;N]
TableScan: TestData projection=[a], partial_filters=[TestData.a =
Int64(1)] [a:Int64;N]
test cases::roundtrip_logical_plan::roundtrip_mixed_case_table_reference ...
ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 112 filtered
out; finished in 0.06s
```
## Are there any user-facing changes?
None.
<!-- Resources -->
[fromcol_proto]:
https://github.com/apache/datafusion/blob/50.2.0/datafusion/proto-common/src/from_proto/mod.rs#L146
[tryfrom_proto]:
https://github.com/apache/datafusion/blob/50.2.0/datafusion/proto-common/src/from_proto/mod.rs#L171
[parse_str]:
https://github.com/apache/datafusion/blob/50.2.0/datafusion/common/src/table_reference.rs#L273
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
