>From Ali Alsuliman <[email protected]>: Attention is currently required from: Vijay Sarathy. Ali Alsuliman has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17404 )
Change subject: [ASTERIXDB-3120][ASTERIXDB-3121] CBO Analyze: improve error message. ...................................................................... Patch Set 3: (6 comments) File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ErrorCode.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17404/comment/ff6be043_9e119089 PS3, Line 270: INVALID_SAMPLE_SIZE_STRING_OPTIONS(1175), this should be removed after you revert the change for it. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17404/comment/85a850db_306481d9 PS3, Line 271: INVALID_SAMPLE_SIZE_NUMBER_OPTIONS change it to INVALID_SAMPLE_SIZE https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17404/comment/74ee1ea9_e9598481 PS3, Line 272: INVALID_SAMPLE_SEED_STRING_OPTIONS change it to INVALID_SAMPLE_SEED File asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17404/comment/bfa8b032_85b2f53f PS3, Line 272: Sample size options are "low", "medium", "high", a positive numeric value, or a string convertible to a positive numeric value let's change it to: Sample size options are "low", "medium", "high", or a number File asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/statement/AnalyzeStatement.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17404/comment/dd08ec87_df46f04b PS3, Line 72: final List<FieldBinding> fbList = options.getFbList(); : for (int i = 0; i < fbList.size(); i++) { looking at this and usages of ExpressionUtils.toNode(), we should just fix ExpressionUtils.toNode() to accept negative, positive numbers. Now that we are making this change. Add a test case where the sample-seed is -ve and +ve. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17404/comment/e7d7dc70_7a67c05a PS3, Line 142: int v; : try { : v = Integer.parseInt(s); : } catch (NumberFormatException e) { : throw new CompilationException(ErrorCode.INVALID_SAMPLE_SIZE_STRING_OPTIONS); : } : if (!isValidSampleSize(v)) { : throw new CompilationException(ErrorCode.INVALID_SAMPLE_SIZE_NUMBER_OPTIONS, : SAMPLE_LOW_SIZE, SAMPLE_HIGH_SIZE); : } : return v; we don't need this. keep it as before where we only accept numbers or "low", "medium" and "high" -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17404 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: neo Gerrit-Change-Id: I599005f4f4814f719e422da4ff7d58c2253e5ed4 Gerrit-Change-Number: 17404 Gerrit-PatchSet: 3 Gerrit-Owner: Vijay Sarathy <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Vijay Sarathy <[email protected]> Gerrit-CC: Anon. E. Moose #1000171 Gerrit-Attention: Vijay Sarathy <[email protected]> Gerrit-Comment-Date: Wed, 01 Mar 2023 19:11:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
