ChinmaySKulkarni commented on a change in pull request #935:
URL: https://github.com/apache/phoenix/pull/935#discussion_r512906673
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
##########
@@ -3048,7 +3048,13 @@ public boolean isViewReferenced() {
* the counter as NULL_COUNTER for extra safety.
*/
EncodedCQCounter cqCounterToBe = tableType == PTableType.VIEW ?
NULL_COUNTER : cqCounter;
- PTable table = new PTableImpl.Builder()
+ PTable table;
+ //better to use the table sent back from the server so we get an
accurate DDL
+ // timestamp, which is server-generated.
+ if (result.getTable() != null ) {
Review comment:
I'm not sure that would be covered by a test since we'd only see it if
the test were to actually look at the cached PTable rather than say doing a
`PhoenixRuntime.getTableNoCache()` right? Since we're changing what is cached,
we should add some tests around this now to make sure the PTable has all
expected fields set.
I see some tests that should cover this code path inside
AlterTableWithViewsIT (like `testAlterPropertiesOfParentTable()`) and also some
in ViewTTLIT, but I'm not sure to what extent they would capture this.
I'm mostly worried about things like setting certain properties based on the
parent PTable or overriding null properties with the default values, etc. (see
[this](https://github.com/apache/phoenix/blob/ac0538b7f5456dd4f91b948c1a51c0042e115955/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java#L3093-L3104)
and
[this](https://github.com/apache/phoenix/blob/ac0538b7f5456dd4f91b948c1a51c0042e115955/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java#L3071-L3074)).
That's where I'm not sure that the PTable returned from the server will be
identical to the one that's created by the client today, since I don't see any
code in MetaDataEndpointImpl that does this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
