praveenc7 commented on code in PR #15350:
URL: https://github.com/apache/pinot/pull/15350#discussion_r2067885886
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOrderByOperator.java:
##########
@@ -279,7 +279,13 @@ private SelectionResultsBlock computePartiallyOrdered() {
int numColumns = columns.size();
Map<String, DataSource> dataSourceMap = new HashMap<>();
for (String column : columns) {
- dataSourceMap.put(column, _indexSegment.getDataSource(column));
+ DataSource dataSource = _indexSegment.getDataSource(column);
+ if (dataSource != null) {
+ dataSourceMap.put(column, dataSource);
+ } else {
+ nonOrderByExpressions.remove(ExpressionContext.forIdentifier(column));
Review Comment:
Added tests , can you describe the 3rd case
##########
pinot-core/src/main/java/org/apache/pinot/core/query/request/context/QueryContext.java:
##########
@@ -495,6 +497,14 @@ public boolean isIndexUseAllowed(String columnName,
FieldConfig.IndexType indexT
return !_skipIndexes.getOrDefault(columnName,
Collections.EMPTY_SET).contains(indexType);
}
+ public void setHasSchemaMisMatch(boolean hasSchemaMismatch) {
Review Comment:
You are right, forgot to cleanup
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java:
##########
@@ -1730,6 +1730,7 @@ static void updateColumnNames(String rawTableName,
PinotQuery pinotQuery, boolea
}
}
//if query has a '*' selection along with other columns
+ pinotQuery.getQueryOptions().put(QueryOptionKey.IS_SELECT_STAR_QUERY,
String.valueOf(hasStar));
Review Comment:
On Second taughts given this path is not accessed in cases when this method
is skipped. Moving it during complication phase to ensure it is always added
from broker
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/ControllerRequestClient.java:
##########
@@ -241,11 +241,12 @@ public String checkIfReloadIsNeeded(String
tableNameWithType, Boolean verbose)
}
}
- public void reloadSegment(String tableName, String segmentName, boolean
forceReload)
+ public String reloadSegment(String tableName, String segmentName, boolean
forceReload)
Review Comment:
Needed for one of the cases in the integ-test that I added. Further this is
mostly used for testing and wanted to keep in parity with the other reload
function we have in the class that returns the jobId :
https://github.com/apache/pinot/pull/15350/files#diff-c00715308b1edd2f6825e58be150660be9d004d21c127f7b6e1eb7fb83f1ab76L222
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java:
##########
@@ -235,6 +237,18 @@ public RelDataType toRelDataType(RelDataTypeFactory
typeFactory) {
return typeFactory.createStructType(columnTypes,
Arrays.asList(_columnNames));
}
+ @JsonIgnore
+ public Map<String, Integer> getColumnNameToIndexMap() {
Review Comment:
Agree that we can do a simple merge sort for sorted list. But I wanted to
avoid that assumption since didn't get a strong indication that blocks would be
in a sorted order.
Further if we change the ordering in the future, we might have to ensure
this block is updated
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java:
##########
@@ -1730,6 +1730,7 @@ static void updateColumnNames(String rawTableName,
PinotQuery pinotQuery, boolea
}
}
//if query has a '*' selection along with other columns
+ pinotQuery.getQueryOptions().put(QueryOptionKey.IS_SELECT_STAR_QUERY,
String.valueOf(hasStar));
Review Comment:
Moved the addition during query compilation
##########
pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java:
##########
@@ -182,10 +187,12 @@ public static Pair<DataSchema, int[]>
getResultTableDataSchemaAndColumnIndices(Q
// No order-by expression
// NOTE: Order-by expressions are ignored for queries with LIMIT 0.
+ boolean isSelectStar = queryContext.getQueryOptions() != null
+ && QueryOptionsUtils.isSelectStarQuery(queryContext.getQueryOptions());
List<OrderByExpressionContext> orderByExpressions =
queryContext.getOrderByExpressions();
if (orderByExpressions == null || queryContext.getLimit() == 0) {
// For 'SELECT *', use the server response data schema as the final
results data schema.
- if ((numSelectExpressions == 1 &&
selectExpressions.get(0).equals(IDENTIFIER_STAR))) {
+ if ((numSelectExpressions == 1 &&
selectExpressions.get(0).equals(IDENTIFIER_STAR) || isSelectStar)) {
Review Comment:
Right, let me remove this, it is only touched by
`BaseSingleStageBrokerRequestHandler` and we are ensuring the selectStar
options would be present if the query has * for this class
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
