palashc commented on code in PR #1883:
URL: https://github.com/apache/phoenix/pull/1883#discussion_r1638639118
##########
phoenix-core-client/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java:
##########
@@ -636,7 +636,7 @@ public MutationPlan compile(UpsertStatement upsert) throws
SQLException {
// as max TS, so that the query can safely restarted and still
work of a snapshot
// (so it won't see its own data in case of concurrent splits)
// see PHOENIX-4849
- long serverTime =
selectResolver.getTables().get(0).getCurrentTime();
+ long serverTime =
selectResolver.getTables().get(0).getTimeStamp();
Review Comment:
@tkhurana Currently both of those methods return the same value i.e.
`upperBoundTimestamp` which is either -1 or whatever is provided to the
TableRef constructor. Current code:
```
// if UPDATE_CACHE_FREQUENCY is set, always let the server set timestamps
this.upperBoundTimeStamp = table.getUpdateCacheFrequency()!=0 ?
QueryConstants.UNSET_TIMESTAMP : upperBoundTimeStamp;
this.currentTime = this.upperBoundTimeStamp;
public long getTimeStamp() {
return this.upperBoundTimeStamp;
}
public long getCurrentTime() {
return this.currentTime;
}
```
Once we set UPDATE_CACHE_FREQ to NEVER, currentTime was being set to -1 and
features using currentTime were breaking (like QueryOptimizer deciding whether
a disabled index is under its usability threshold or asyncCreatedDate during
CreateIndex). The following change in TableRef keeps currentTime to whatever is
provided to the TableRef constructor so that features using currentTime can
continue to use it. There is effectively no change in UpsertCompiler - it will
still be using the same value i.e. TableRef.upperBoundTimeStamp.
```
this.currentTime = upperBoundTimeStamp;
```
--
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]
