Support REFRESH MATERIALIZED VIEW

[gggrace14]

Depended by https://github.com/facebookexternal/presto-facebook/pull/1509

[highker]

Let's rebase this PR on top of #15910. That one is one a shape that won't have major interface or structure change.

[gggrace14]

Let's rebase this PR on top of #15910. That one is one a shape that won't have major interface or structure change.

I will do it and come back.

[highker]

Quick comments on the first commit

[jainxrohit]

Have only looked at the query building part of it, have left some comments for clarification.

[highker]

Let's clean up the initial comments, rebase, and have another round of review

[jainxrohit]

Can we please resolve the the comments which are already addressed?

[jainxrohit]

Left the change in comments

[gggrace14]

Addressed comments, and also changed TestHiveLogicalPlanner to use REFRESH MV to write to MVs and enabled the test cases.

[jainxrohit]

@gggrace14 are we planning to fix testMaterializedViewForJoin test as part of this PR?
If not can we create a new task and update the TODO?

I see that testMaterializedViewForJoinWithMultiplePartitions as well is disabled. Can we enable it if there are no issues?

[jainxrohit]

Overall it looks good to me. Lets make the final set of requested changes.

I would have preferred to keep the query rewriting logic at one place, as it is easier to read and debug. But may be we can create a task for refactoring it.

cc: @highker

[gggrace14]

@gggrace14 are we planning to fix testMaterializedViewForJoin test as part of this PR?
If not can we create a new task and update the TODO?

I see that testMaterializedViewForJoinWithMultiplePartitions as well is disabled. Can we enable it if there are no issues?

Yes, testMaterializedViewForJoinWithMultiplePartitions is enabled now and it is good with replacing insert w/ refresh. Fixing testMaterializedViewForJoin needs more complete population of the columnMappings structure, which is orthogonal to REFRESH and deserves a standalone PR. Filing the issue #16220 and updating the TODO.

[jainxrohit]

testMaterializedViewForJoinWithMultiplePartitions

I was also talking about test method
public void testMaterializedViewForJoin()

[gggrace14]

@gggrace14 are we planning to fix testMaterializedViewForJoin test as part of this PR?
If not can we create a new task and update the TODO?

I see that testMaterializedViewForJoinWithMultiplePartitions as well is disabled. Can we enable it if there are no issues?

Yes, testMaterializedViewForJoinWithMultiplePartitions is enabled now and it is good with replacing insert w/ refresh.

[gggrace14]

testMaterializedViewForJoinWithMultiplePartitions

I was also talking about test method
public void testMaterializedViewForJoin()

Filing the issue #16220 and updating the TODO.

[highker]

Remove this interface. Let's only keep the constructor(s)

[highker]

Use the constructor instead of withXXX

[highker]

No need to call it viewDefinitionWithRefreshColumns, just reuse viewDefinition

[highker]

merge into one line

[highker]

typo: REFERSH

[highker]

Let's keep these unchanged

[ajaygeorge]

@gggrace14 There are no release notes on this PR. Can you please add them.
cc @highker / @jainxrohit

[jainxrohit]

@gggrace14 There are no release notes on this PR. Can you please add them.
cc @highker / @jainxrohit

My bad, @gggrace14, let add no release notes for this PR.

[gggrace14]

Yes, will do!