[
https://issues.apache.org/jira/browse/PHOENIX-6186?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17221614#comment-17221614
]
ASF GitHub Bot commented on PHOENIX-6186:
-----------------------------------------
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]
> Store table metadata last modified timestamp in PTable / System.Catalog
> -----------------------------------------------------------------------
>
> Key: PHOENIX-6186
> URL: https://issues.apache.org/jira/browse/PHOENIX-6186
> Project: Phoenix
> Issue Type: New Feature
> Reporter: Geoffrey Jacoby
> Assignee: Geoffrey Jacoby
> Priority: Major
> Fix For: 4.16.0
>
> Attachments: PHOENIX-6186-4.x.patch
>
>
> There are many reasons why it's useful to know when a particular table's
> metadata was last modified. It's helpful when solving cache coherency
> problems, and also in order to interact with external schema registries which
> may have multiple versions of a particular schema and require a timestamp to
> resolve ambiguities.
> This JIRA will add a last modified timestamp field to System.Catalog, to be
> updated both when creating a table/view and also when adding or removing a
> column. Changing purely internal Phoenix properties will not update the
> timestamp.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
