mcvsubbu commented on a change in pull request #8036:
URL: https://github.com/apache/pinot/pull/8036#discussion_r839056621
##########
File path: compatibility-verifier/sample-test-suite/post-server-rollback.yaml
##########
@@ -44,3 +58,14 @@ operations:
description: Run query on FeatureTest2 using SQL
queryFileName: queries/feature-test-2-sql-realtime.queries
expectedResultsFileName: query-results/feature-test-2-sql-realtime.results
+ - type: queryOp
+ description: Run query on FeatureTest3 using SQL
+ queryFileName: queries/feature-test-3-sql-realtime.queries
+ expectedResultsFileName: query-results/feature-test-3-sql-realtime.results
+ shouldCompareNumEntriesScannedInFilter: true
Review comment:
we should not need these additional fields anymore right?
##########
File path:
pinot-compatibility-verifier/src/main/java/org/apache/pinot/compat/QueryOp.java
##########
@@ -72,16 +75,44 @@ public void setExpectedResultsFileName(String
expectedResultsFileName) {
_expectedResultsFileName = expectedResultsFileName;
}
+ public Boolean getShouldCompareNumEntriesScannedInFilter() {
+ return _shouldCompareNumEntriesScannedInFilter;
+ }
+
+ public void setShouldCompareNumEntriesScannedInFilter(Boolean
shouldCompareNumEntriesScannedInFilter) {
+ _shouldCompareNumEntriesScannedInFilter =
shouldCompareNumEntriesScannedInFilter;
+ }
+
+ public Boolean getShouldCompareNumConsumingSegmentsQueried() {
+ return _shouldCompareNumConsumingSegmentsQueried;
+ }
+
+ public void setShouldCompareNumConsumingSegmentsQueried(boolean
shouldCompareNumConsumingSegmentsQueried) {
+ _shouldCompareNumConsumingSegmentsQueried =
shouldCompareNumConsumingSegmentsQueried;
+ }
+
+ public Boolean getShouldRunAllGenerations() {
+ return _shouldRunAllGenerations;
+ }
+
+ public void setShouldRunAllGenerations(Boolean shouldRunAllGenerations) {
+ _shouldRunAllGenerations = shouldRunAllGenerations;
+ }
+
@Override
boolean runOp(int generationNumber) {
System.out.println("Verifying queries in " + _queryFileName + " against
results in " + _expectedResultsFileName);
try {
- for (int i = 1; i <= generationNumber; i++) {
- if (!verifyQueries(i)) {
- return false;
+ if (getShouldRunAllGenerations()) {
Review comment:
I don't understand why we should not be running all generations.
If we delete and create the table everytime, then it need not be added in
compatibility test. The whole idea of compatibility testing is that we can work
with tables in which data was added in previous versions, or, while
downgrading, the data was added in new versions.
##########
File path: compatibility-verifier/sample-test-suite/post-server-rollback.yaml
##########
@@ -44,3 +58,14 @@ operations:
description: Run query on FeatureTest2 using SQL
queryFileName: queries/feature-test-2-sql-realtime.queries
expectedResultsFileName: query-results/feature-test-2-sql-realtime.results
+ - type: queryOp
+ description: Run query on FeatureTest3 using SQL
+ queryFileName: queries/feature-test-3-sql-realtime.queries
+ expectedResultsFileName: query-results/feature-test-3-sql-realtime.results
+ shouldCompareNumEntriesScannedInFilter: true
+ shouldCompareNumConsumingSegmentsQueried: true
Review comment:
Why do we need this?
##########
File path: compatibility-verifier/sample-test-suite/post-broker-rollback.yaml
##########
@@ -20,6 +20,12 @@
# Operations to be done.
description: Operations to be run after broker rollback
operations:
+ - type: tableOp
Review comment:
We can set the numEntriesscanned to a high value (my recent PR)
Also, I believe that we execute queries for a specific generation number, so
at any one run, only one gen num worth of rows will be considered.
##########
File path: compatibility-verifier/sample-test-suite/config/dataGenerator.py
##########
@@ -0,0 +1,90 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+import random
+import csv
+import string
+
+class DataGenerator:
Review comment:
I think this is used to generate featuretest3? Is it one time (I hope)
or on each run (in which case I have more questions)? Either way, please add a
comment on this file. Thanks.
--
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
