walterddr commented on code in PR #9561:
URL: https://github.com/apache/pinot/pull/9561#discussion_r990822348
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/InstanceResponseBlock.java:
##########
@@ -18,26 +18,132 @@
*/
package org.apache.pinot.core.operator.blocks;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.annotation.Nullable;
import org.apache.pinot.common.datatable.DataTable;
+import org.apache.pinot.common.response.ProcessingException;
+import org.apache.pinot.common.utils.DataSchema;
import org.apache.pinot.core.common.Block;
import org.apache.pinot.core.common.BlockDocIdSet;
import org.apache.pinot.core.common.BlockDocIdValueSet;
import org.apache.pinot.core.common.BlockMetadata;
import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.common.datatable.DataTableBuilderFactory;
+import org.apache.pinot.core.operator.blocks.results.BaseResultsBlock;
+import org.apache.pinot.core.query.request.context.QueryContext;
/**
- * InstanceResponseBlock is just a holder to get InstanceResponse from
InstanceResponseBlock.
+ * The {@code InstanceResponseBlock} is the holder of the server side results.
*/
public class InstanceResponseBlock implements Block {
- private final DataTable _instanceResponseDataTable;
+ private final BaseResultsBlock _resultsBlock;
+ private final QueryContext _queryContext;
+ private final Map<Integer, String> _exceptions;
+ private final Map<String, String> _metadata;
- public InstanceResponseBlock(DataTable dataTable) {
- _instanceResponseDataTable = dataTable;
+ public InstanceResponseBlock(BaseResultsBlock resultsBlock, QueryContext
queryContext) {
+ _resultsBlock = resultsBlock;
+ _queryContext = queryContext;
+ _exceptions = new HashMap<>();
+ List<ProcessingException> processingExceptions =
resultsBlock.getProcessingExceptions();
Review Comment:
(maybe in another PR) can we change BaseResultBlock to save QueryException?
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/InstanceResponseBlock.java:
##########
@@ -18,26 +18,132 @@
*/
package org.apache.pinot.core.operator.blocks;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.annotation.Nullable;
import org.apache.pinot.common.datatable.DataTable;
+import org.apache.pinot.common.response.ProcessingException;
+import org.apache.pinot.common.utils.DataSchema;
import org.apache.pinot.core.common.Block;
import org.apache.pinot.core.common.BlockDocIdSet;
import org.apache.pinot.core.common.BlockDocIdValueSet;
import org.apache.pinot.core.common.BlockMetadata;
import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.common.datatable.DataTableBuilderFactory;
+import org.apache.pinot.core.operator.blocks.results.BaseResultsBlock;
+import org.apache.pinot.core.query.request.context.QueryContext;
/**
- * InstanceResponseBlock is just a holder to get InstanceResponse from
InstanceResponseBlock.
+ * The {@code InstanceResponseBlock} is the holder of the server side results.
*/
public class InstanceResponseBlock implements Block {
- private final DataTable _instanceResponseDataTable;
+ private final BaseResultsBlock _resultsBlock;
+ private final QueryContext _queryContext;
+ private final Map<Integer, String> _exceptions;
+ private final Map<String, String> _metadata;
- public InstanceResponseBlock(DataTable dataTable) {
- _instanceResponseDataTable = dataTable;
+ public InstanceResponseBlock(BaseResultsBlock resultsBlock, QueryContext
queryContext) {
+ _resultsBlock = resultsBlock;
+ _queryContext = queryContext;
+ _exceptions = new HashMap<>();
+ List<ProcessingException> processingExceptions =
resultsBlock.getProcessingExceptions();
+ if (processingExceptions != null) {
+ for (ProcessingException processingException : processingExceptions) {
+ _exceptions.put(processingException.getErrorCode(),
processingException.getMessage());
+ }
+ }
+ _metadata = resultsBlock.getResultsMetadata();
}
- public DataTable getInstanceResponseDataTable() {
- return _instanceResponseDataTable;
+ /**
+ * Metadata only instance response.
+ */
+ public InstanceResponseBlock() {
+ _resultsBlock = null;
+ _queryContext = null;
+ _exceptions = new HashMap<>();
+ _metadata = new HashMap<>();
+ }
+
+ private InstanceResponseBlock(Map<Integer, String> exceptions, Map<String,
String> metadata) {
+ _resultsBlock = null;
+ _queryContext = null;
+ _exceptions = exceptions;
+ _metadata = metadata;
+ }
+
+ public InstanceResponseBlock toMetadataOnlyResponseBlock() {
+ return new InstanceResponseBlock(_exceptions, _metadata);
+ }
+
+ public void addException(ProcessingException processingException) {
+ _exceptions.put(processingException.getErrorCode(),
processingException.getMessage());
+ }
+
+ public void addException(int errorCode, String exceptionMessage) {
+ _exceptions.put(errorCode, exceptionMessage);
+ }
+
+ public void addMetadata(String key, String value) {
+ _metadata.put(key, value);
+ }
+
+ @Nullable
+ public BaseResultsBlock getResultsBlock() {
+ return _resultsBlock;
+ }
+
+ @Nullable
+ public QueryContext getQueryContext() {
+ return _queryContext;
+ }
+
+ public Map<Integer, String> getExceptions() {
+ return _exceptions;
+ }
+
+ public Map<String, String> getResponseMetadata() {
+ return _metadata;
+ }
+
+ @Nullable
+ public DataSchema getDataSchema() {
+ return _resultsBlock != null ? _resultsBlock.getDataSchema(_queryContext)
: null;
+ }
+
+ @Nullable
+ public Collection<Object[]> getRows() {
+ return _resultsBlock != null ? _resultsBlock.getRows(_queryContext) : null;
+ }
+
+ public DataTable toDataTable()
+ throws IOException {
+ DataTable dataTable = toDataOnlyDataTable();
+ attachMetadata(dataTable);
+ return dataTable;
+ }
Review Comment:
is there any situation one will use the BaseResultsBlock API rather than the
ones in InstanceResponseBlock?
if not can we make all the abstract methods in BaseResultsBlock package
private and move the InstanceResponseBlock into the results package?
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/InstanceResponseBlock.java:
##########
@@ -18,26 +18,132 @@
*/
package org.apache.pinot.core.operator.blocks;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.annotation.Nullable;
import org.apache.pinot.common.datatable.DataTable;
+import org.apache.pinot.common.response.ProcessingException;
+import org.apache.pinot.common.utils.DataSchema;
import org.apache.pinot.core.common.Block;
import org.apache.pinot.core.common.BlockDocIdSet;
import org.apache.pinot.core.common.BlockDocIdValueSet;
import org.apache.pinot.core.common.BlockMetadata;
import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.common.datatable.DataTableBuilderFactory;
+import org.apache.pinot.core.operator.blocks.results.BaseResultsBlock;
+import org.apache.pinot.core.query.request.context.QueryContext;
/**
- * InstanceResponseBlock is just a holder to get InstanceResponse from
InstanceResponseBlock.
+ * The {@code InstanceResponseBlock} is the holder of the server side results.
*/
public class InstanceResponseBlock implements Block {
- private final DataTable _instanceResponseDataTable;
+ private final BaseResultsBlock _resultsBlock;
+ private final QueryContext _queryContext;
+ private final Map<Integer, String> _exceptions;
+ private final Map<String, String> _metadata;
- public InstanceResponseBlock(DataTable dataTable) {
- _instanceResponseDataTable = dataTable;
+ public InstanceResponseBlock(BaseResultsBlock resultsBlock, QueryContext
queryContext) {
+ _resultsBlock = resultsBlock;
+ _queryContext = queryContext;
+ _exceptions = new HashMap<>();
+ List<ProcessingException> processingExceptions =
resultsBlock.getProcessingExceptions();
+ if (processingExceptions != null) {
+ for (ProcessingException processingException : processingExceptions) {
+ _exceptions.put(processingException.getErrorCode(),
processingException.getMessage());
+ }
+ }
+ _metadata = resultsBlock.getResultsMetadata();
Review Comment:
do we guarantee that resultsBlock.getResultsMetadata() will return constant
result / will not change after it is being added to InstanceResponseBlock?
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
