You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by st...@apache.org on 2021/05/10 03:52:08 UTC

[phoenix] branch master updated: PHOENIX-6357 Change all command line tools to use the fixed commons-cli constructor

This is an automated email from the ASF dual-hosted git repository.

stoty pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/master by this push:
     new 88a0128  PHOENIX-6357 Change all command line tools to use the fixed commons-cli constructor
88a0128 is described below

commit 88a0128064ce8be768266f9804e4829ef87af785
Author: Istvan Toth <st...@apache.org>
AuthorDate: Thu Mar 25 10:25:57 2021 +0100

    PHOENIX-6357 Change all command line tools to use the fixed commons-cli constructor
    
    use the new DefaultParser(false, false) constructor introduced in the patched
    phoenix-thirdparty commons-cli library, to make sure that any quoted identifiers
    are passed from the command line to the rest of the code correctly.
    
    remove the double doublequote partial workaround, as it is no longer needed
---
 .../apache/phoenix/end2end/CsvBulkLoadToolIT.java  |  8 +++---
 .../phoenix/mapreduce/AbstractBulkLoadTool.java    | 32 ++--------------------
 .../apache/phoenix/mapreduce/OrphanViewTool.java   |  4 +--
 .../apache/phoenix/mapreduce/PhoenixTTLTool.java   |  4 +--
 .../phoenix/mapreduce/index/IndexScrutinyTool.java |  2 +-
 .../apache/phoenix/mapreduce/index/IndexTool.java  |  2 +-
 .../phoenix/mapreduce/index/IndexUpgradeTool.java  |  3 +-
 .../phoenix/schema/stats/UpdateStatisticsTool.java |  4 +--
 .../util/MergeViewIndexIdSequencesTool.java        |  4 +--
 .../org/apache/phoenix/util/PhoenixRuntime.java    |  4 +--
 .../main/java/org/apache/phoenix/pherf/Pherf.java  |  4 +--
 .../phoenix/schema/SchemaExtractionTool.java       |  4 +--
 12 files changed, 23 insertions(+), 52 deletions(-)

diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CsvBulkLoadToolIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CsvBulkLoadToolIT.java
index 0a34ca0..b569d44 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CsvBulkLoadToolIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CsvBulkLoadToolIT.java
@@ -206,8 +206,8 @@ public class CsvBulkLoadToolIT extends BaseOwnClusterIT {
                 "--input", "/tmp/input8.csv",
                 "--table", "table8",
                 "--zookeeper", zkQuorum,
-                "-q", "\"\"",
-                "-e", "\"\"",
+                "-q", "",
+                "-e", "",
                 "--delimiter", "\\t"
                 });
         assertEquals(0, exitCode);
@@ -701,7 +701,7 @@ public class CsvBulkLoadToolIT extends BaseOwnClusterIT {
         csvBulkLoadTool.getConf().set(DATE_FORMAT_ATTRIB,"yyyy/MM/dd");
         int exitCode = csvBulkLoadTool.run(new String[] {
                 "--input", "/tmp/input1.csv",
-                "--table", "\"\"t\"\"",
+                "--table", "\"t\"",
                 "--schema", "S",
                 "--zookeeper", zkQuorum});
         assertEquals(0, exitCode);
@@ -736,7 +736,7 @@ public class CsvBulkLoadToolIT extends BaseOwnClusterIT {
         int exitCode = csvBulkLoadTool.run(new String[] {
                 "--input", "/tmp/input1.csv",
                 "--table", "T",
-                "--schema", "\"\"s\"\"",
+                "--schema", "\"s\"",
                 "--zookeeper", zkQuorum});
         assertEquals(0, exitCode);
         ResultSet rs = stmt.executeQuery("SELECT id, name, t FROM \"s\".T ORDER BY id");
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/AbstractBulkLoadTool.java b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/AbstractBulkLoadTool.java
index 4224e67..b777fbc 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/AbstractBulkLoadTool.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/AbstractBulkLoadTool.java
@@ -126,7 +126,7 @@ public abstract class AbstractBulkLoadTool extends Configured implements Tool {
 
         Options options = getOptions();
 
-        CommandLineParser parser = new DefaultParser();
+        CommandLineParser parser = new DefaultParser(false, false);
         CommandLine cmdLine = null;
         try {
             cmdLine = parser.parse(options, args);
@@ -181,40 +181,12 @@ public abstract class AbstractBulkLoadTool extends Configured implements Tool {
         return loadData(conf, cmdLine);
     }
 
-    /**
-     * Check schema or table name that start with two double quotes i.e ""t"" -> true
-     */
-    private boolean isStartWithTwoDoubleQuotes (String name) {
-        boolean start = false;
-        boolean end = false;
-        if (name != null && name.length() > 1) {
-             int length = name.length();
-             start = name.substring(0,2).equals("\"\"");
-             end =  name.substring(length-2, length).equals("\"\"");
-             if (start && !end) {
-                 throw new IllegalArgumentException("Invalid table/schema name " + name +
-                         ". Please check if name end with two double quotes.");
-             }
-        }
-        return start;
-    }
-
 
     private int loadData(Configuration conf, CommandLine cmdLine) throws Exception {
         String tableName = cmdLine.getOptionValue(TABLE_NAME_OPT.getOpt());
         String schemaName = cmdLine.getOptionValue(SCHEMA_NAME_OPT.getOpt());
         String indexTableName = cmdLine.getOptionValue(INDEX_TABLE_NAME_OPT.getOpt());
-        boolean quotedTableName = isStartWithTwoDoubleQuotes(tableName);
-        if (quotedTableName) {
-            // Commons-cli cannot parse full quoted argument i.e "t" (CLI-275).
-            // if \"\"t\"\" passed, then both pairs of quoted are left intact as ""t"".
-            // So remove one pair of quote from tablename ""t"" -> "t".
-            tableName = tableName.substring(1, tableName.length() - 1);
-        }
-        boolean quotedSchemaName = isStartWithTwoDoubleQuotes(schemaName);
-        if (quotedSchemaName) {
-            schemaName = schemaName.substring(1,schemaName.length() - 1);
-        }
+
         String qualifiedTableName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
         String qualifiedIndexTableName = null;
         if (indexTableName != null){
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/OrphanViewTool.java b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/OrphanViewTool.java
index cdd6650..563d69b 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/OrphanViewTool.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/OrphanViewTool.java
@@ -49,11 +49,11 @@ import java.util.Properties;
 
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLine;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLineParser;
+import org.apache.phoenix.thirdparty.org.apache.commons.cli.DefaultParser;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.HelpFormatter;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Option;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Options;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.ParseException;
-import org.apache.phoenix.thirdparty.org.apache.commons.cli.PosixParser;
 import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.conf.Configured;
@@ -194,7 +194,7 @@ public class OrphanViewTool extends Configured implements Tool {
 
         final Options options = getOptions();
 
-        CommandLineParser parser = new PosixParser();
+        CommandLineParser parser = new DefaultParser(false, false);
         CommandLine cmdLine = null;
         try {
             cmdLine = parser.parse(options, args);
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/PhoenixTTLTool.java b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/PhoenixTTLTool.java
index 1960d6d..0f43264 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/PhoenixTTLTool.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/PhoenixTTLTool.java
@@ -19,10 +19,10 @@ package org.apache.phoenix.mapreduce;
 
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLine;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLineParser;
+import org.apache.phoenix.thirdparty.org.apache.commons.cli.DefaultParser;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Option;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Options;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.HelpFormatter;
-import org.apache.phoenix.thirdparty.org.apache.commons.cli.PosixParser;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.ParseException;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.conf.Configured;
@@ -173,7 +173,7 @@ public class PhoenixTTLTool extends Configured implements Tool {
 
     public CommandLine parseOptions(String[] args) {
         final Options options = getOptions();
-        CommandLineParser parser = new PosixParser();
+        CommandLineParser parser = new DefaultParser(false, false);
         CommandLine cmdLine = null;
         try {
             cmdLine = parser.parse(options, args);
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyTool.java b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyTool.java
index dd7b628..ed04865 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyTool.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyTool.java
@@ -170,7 +170,7 @@ public class IndexScrutinyTool extends Configured implements Tool {
     private CommandLine parseOptions(String[] args) {
         final Options options = getOptions();
 
-        CommandLineParser parser = new DefaultParser();
+        CommandLineParser parser = new DefaultParser(false, false);
         CommandLine cmdLine = null;
         try {
             cmdLine = parser.parse(options, args);
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexTool.java b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexTool.java
index b9b77bd..fbb9b18 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexTool.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexTool.java
@@ -353,7 +353,7 @@ public class IndexTool extends Configured implements Tool {
 
         final Options options = getOptions();
 
-        CommandLineParser parser = new DefaultParser();
+        CommandLineParser parser = new DefaultParser(false, false);
         CommandLine cmdLine = null;
         try {
             cmdLine = parser.parse(options, args);
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java
index 31f81ee..ba7d957 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java
@@ -26,7 +26,6 @@ import org.apache.phoenix.thirdparty.org.apache.commons.cli.HelpFormatter;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Option;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Options;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.ParseException;
-import org.apache.phoenix.thirdparty.org.apache.commons.cli.PosixParser;
 import org.apache.hadoop.conf.Configured;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.client.CoprocessorDescriptorBuilder;
@@ -229,7 +228,7 @@ public class IndexUpgradeTool extends Configured implements Tool {
 
         final Options options = getOptions();
 
-        CommandLineParser parser = new PosixParser();
+        CommandLineParser parser = new DefaultParser(false, false);
         CommandLine cmdLine = null;
         try {
             cmdLine = parser.parse(options, args);
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/stats/UpdateStatisticsTool.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/stats/UpdateStatisticsTool.java
index 66ffac1..9aff6ac 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/schema/stats/UpdateStatisticsTool.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/stats/UpdateStatisticsTool.java
@@ -20,11 +20,11 @@ package org.apache.phoenix.schema.stats;
 import org.antlr.runtime.CharStream;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLine;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLineParser;
+import org.apache.phoenix.thirdparty.org.apache.commons.cli.DefaultParser;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.HelpFormatter;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Option;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Options;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.ParseException;
-import org.apache.phoenix.thirdparty.org.apache.commons.cli.PosixParser;
 import org.apache.hadoop.conf.Configured;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.HBaseConfiguration;
@@ -265,7 +265,7 @@ public class UpdateStatisticsTool extends Configured implements Tool {
 
         final Options options = getOptions();
 
-        CommandLineParser parser = new PosixParser();
+        CommandLineParser parser = new DefaultParser(false, false);
         CommandLine cmdLine = null;
         try {
             cmdLine = parser.parse(options, args);
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/MergeViewIndexIdSequencesTool.java b/phoenix-core/src/main/java/org/apache/phoenix/util/MergeViewIndexIdSequencesTool.java
index bd798ab..3a15432 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/MergeViewIndexIdSequencesTool.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/MergeViewIndexIdSequencesTool.java
@@ -19,11 +19,11 @@ package org.apache.phoenix.util;
 
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLine;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLineParser;
+import org.apache.phoenix.thirdparty.org.apache.commons.cli.DefaultParser;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.HelpFormatter;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Option;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Options;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.ParseException;
-import org.apache.phoenix.thirdparty.org.apache.commons.cli.PosixParser;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.conf.Configured;
 import org.apache.hadoop.hbase.HBaseConfiguration;
@@ -55,7 +55,7 @@ public class MergeViewIndexIdSequencesTool extends Configured implements Tool {
 
         final Options options = getOptions();
 
-        CommandLineParser parser = new PosixParser();
+        CommandLineParser parser = new DefaultParser(false, false);
         CommandLine cmdLine = null;
         try {
             cmdLine = parser.parse(options, args);
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java b/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java
index 3ca925c..d683f38 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java
@@ -46,11 +46,11 @@ import javax.annotation.Nullable;
 
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLine;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLineParser;
+import org.apache.phoenix.thirdparty.org.apache.commons.cli.DefaultParser;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.HelpFormatter;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Option;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Options;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.ParseException;
-import org.apache.phoenix.thirdparty.org.apache.commons.cli.PosixParser;
 import org.apache.commons.lang3.StringEscapeUtils;
 import org.apache.hadoop.hbase.Cell;
 import org.apache.hadoop.hbase.HConstants;
@@ -701,7 +701,7 @@ public class PhoenixRuntime {
             options.addOption(localIndexUpgradeOption);
             options.addOption(binaryEncodingOption);
 
-            CommandLineParser parser = new PosixParser();
+            CommandLineParser parser = new DefaultParser(false, false);
             CommandLine cmdLine = null;
             try {
                 cmdLine = parser.parse(options, args);
diff --git a/phoenix-pherf/src/main/java/org/apache/phoenix/pherf/Pherf.java b/phoenix-pherf/src/main/java/org/apache/phoenix/pherf/Pherf.java
index a9840f9..48eb0f3 100644
--- a/phoenix-pherf/src/main/java/org/apache/phoenix/pherf/Pherf.java
+++ b/phoenix-pherf/src/main/java/org/apache/phoenix/pherf/Pherf.java
@@ -28,10 +28,10 @@ import org.apache.phoenix.thirdparty.com.google.common.annotations.VisibleForTes
 import org.apache.phoenix.thirdparty.com.google.common.collect.Lists;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLine;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLineParser;
+import org.apache.phoenix.thirdparty.org.apache.commons.cli.DefaultParser;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.HelpFormatter;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Options;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.ParseException;
-import org.apache.phoenix.thirdparty.org.apache.commons.cli.PosixParser;
 import org.apache.phoenix.pherf.PherfConstants.CompareType;
 import org.apache.phoenix.pherf.PherfConstants.GeneratePhoenixStats;
 import org.apache.phoenix.pherf.configuration.DataModel;
@@ -132,7 +132,7 @@ public class Pherf {
     WorkloadExecutor workloadExecutor;
 
     public Pherf(String[] args) throws Exception {
-        CommandLineParser parser = new PosixParser();
+        CommandLineParser parser = new DefaultParser(false, false);
         CommandLine command = null;
         HelpFormatter hf = new HelpFormatter();
 
diff --git a/phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionTool.java b/phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionTool.java
index 3807e3c..15dca25 100644
--- a/phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionTool.java
+++ b/phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionTool.java
@@ -19,11 +19,11 @@ package org.apache.phoenix.schema;
 
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLine;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLineParser;
+import org.apache.phoenix.thirdparty.org.apache.commons.cli.DefaultParser;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.HelpFormatter;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Option;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.Options;
 import org.apache.phoenix.thirdparty.org.apache.commons.cli.ParseException;
-import org.apache.phoenix.thirdparty.org.apache.commons.cli.PosixParser;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.conf.Configured;
@@ -83,7 +83,7 @@ public class SchemaExtractionTool extends Configured implements Tool {
 
     private CommandLine parseOptions(String[] args) {
         final Options options = getOptions();
-        CommandLineParser parser = new PosixParser();
+        CommandLineParser parser = new DefaultParser(false, false);
         CommandLine cmdLine = null;
         try {
             cmdLine = parser.parse(options, args);