ajantha-bhat commented on a change in pull request #3907:
URL: https://github.com/apache/carbondata/pull/3907#discussion_r481221931
##########
File path:
core/src/main/java/org/apache/carbondata/core/index/IndexStoreManager.java
##########
@@ -515,15 +516,16 @@ public TableSegmentRefresher
getTableSegmentRefresher(CarbonTable table) {
UpdateVO updateVO =
SegmentUpdateStatusManager.getInvalidTimestampRange(segment.getLoadMetadataDetails());
SegmentRefreshInfo segmentRefreshInfo;
- if ((updateVO != null && updateVO.getLatestUpdateTimestamp() != null)
- || segment.getSegmentFileName() != null) {
- long segmentFileTimeStamp;
+ if (updateVO.getLatestUpdateTimestamp() != null ||
segment.getSegmentFileName() != null) {
+ long segmentFileTimeStamp = 0L;
if (null != segment.getLoadMetadataDetails()) {
segmentFileTimeStamp =
segment.getLoadMetadataDetails().getLastModifiedTime();
} else {
- segmentFileTimeStamp = FileFactory.getCarbonFile(CarbonTablePath
- .getSegmentFilePath(table.getTablePath(),
segment.getSegmentFileName()))
- .getLastModifiedTime();
+ CarbonFile segmentFile = FileFactory.getCarbonFile(CarbonTablePath
+ .getSegmentFilePath(table.getTablePath(),
segment.getSegmentFileName()));
+ if (segmentFile.exists()) {
Review comment:
> can refer PR #3814
Seems that PR is referring from load meta details, it doesn't mention when
it is null
> we cannot differentiate between positive case and negative case
Agree, but instead of adding file exist check , may acquire segment lock (to
make sure segment file there) or make sure load metadetails is filled always so
it wont have to check file exist. Else many places we have to add file exist
check where and all we are accessing the segment
----------------------------------------------------------------
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:
us...@infra.apache.org
