Avoid type coercion when a dereferenced field is same type

[laurachenyu]

Got failure 'Unsupported coercion from int to int' because unnecessary type coercion was called when retrieving a dereferenced field without type changes.

[tooptoop4]

@laurachenyu have you got a sample table definition and query that replicates the issue?

[laurachenyu]

@laurachenyu have you got a sample table definition and query that replicates the issue?

Following test will recreate the issue:

DROP TABLE IF EXISTS evolve_test;
CREATE TABLE evolve_test (dummy bigint, a row(b bigint, c varchar), d bigint) with (format ='orc', partitioned_by=array['d']);
INSERT INTO evolve_test values (1, row(1, 'abc'), 1);
ALTER TABLE evolve_test DROP COLUMN a;
ALTER TABLE evolve_test ADD COLUMN a row(b bigint, c varchar, f int);
INSERT INTO evolve_test values (2, row(2, 'def', 2), 2);
SELECT a.b FROM evolve_test;

[findepi]

@laurachenyu thanks for your PR!

please add a test. We should be able to test this with TestOrcReader (let us know if you need guidance).

If not, we could cover this with a product test (like TestAvroSchemaEvolution)

[phd3]

@laurachenyu Thanks for fixing this issue!

It may be easier to add the suggested testcase here along with other dereference related schema mismatches: https://github.com/prestosql/presto/blob/master/presto-hive/src/test/java/io/prestosql/plugin/hive/TestHiveIntegrationSmokeTest.java#L4230

[laurachenyu]

Thanks, @phd3.

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Laura Chen.
This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email email@example.com
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[phd3]

LGTM % a comment about the comment.

Failure is unrelated, tests've been retriggered.

[phd3]

@cla-bot check

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[phd3]

It seems that we need a similar fix in HiveCoercionRecordCursor too. This RecordCursor does not create the coercers correctly (only assumes top level types). The issue is visible for TEXTFILE input format and s3_select_pushdown_enabled set to true.

The issue is relatively less likely to appear because (1) such a schema evolution itself might be rare and (2) HiveCoercionRecordCursor is not used for GenericHiveRecordCursor. (GenericHiveRecordCursor does the coercion for top-level row.) (3) Also, the CSV format does not face this issue because it only deals with VARCHAR.

[ebyhr]

Could you shorten the commit message as follows or something?

Avoid type coercion when a dereferenced field is same type

@phd3 Let's separate an issue because this PR itself is valuable and actually our environment is facing this type coercion error too.

[ebyhr]

@laurachenyu My above comment meant a commit e617e50 (not PR title). Could you amend the commit message and do force-push?

[laurachenyu]

@laurachenyu My above comment meant a commit e617e50 (not PR title). Could you amend the commit message and do force-push?

@ebyhr Thanks, just pushed with changed commit message.

[ebyhr]

Merged, thanks!