Thespica commented on PR #912: URL: https://github.com/apache/incubator-graphar/pull/912#issuecomment-4192729631
btw, I don’t think this test covers the intended behavior yet. In `Test CastTableWithSchema with string to large_string conversion`, the test builds `original_table` and `target_schema`, but it never actually calls the reader path or `CastTableWithSchema`. The section ends with `SUCCEED(...)`, so it will pass even if the `string -> large_string` conversion is broken. Also, in `Vertex property reader with string data`, the assertion accepts both `arrow::utf8()` and `arrow::large_utf8()`. Since the expected GraphAr schema for `Type::STRING` is `arrow::large_utf8()`, allowing `arrow::utf8()` means the test would still pass even if the conversion did not happen. So at the moment this PR adds test scaffolding, but it does not really verify the `CastTableWithSchema` string-to-large_string conversion path. I think the test should assert the final column type is specifically `arrow::large_utf8()` and exercise a code path that actually triggers the schema cast. -- 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]
