Manno15 commented on a change in pull request #2255:
URL: https://github.com/apache/accumulo/pull/2255#discussion_r709252455
##########
File path:
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java
##########
@@ -347,7 +385,11 @@ public Options getOptions() {
"make end row exclusive (by default it's inclusive)");
optEndRowExclusive.setArgName("end-exclusive");
scanOptRow = new Option("r", "row", true, "row to scan");
- scanOptColumns = new Option("c", "columns", true, "comma-separated
columns");
+ scanOptColumns = new Option("c", "columns", true,
+ "comma-separated columns.This" + "option is mutually exclusive with
cf");
+ scanOptCf = new Option("cf", "column-family", true, "column family to
scan.");
+ scanOptCq = new Option("cq", "column-qualifier", true, "column qualifier");
Review comment:
```suggestion
scanOptCq = new Option("cq", "column-qualifier", true, "column qualifier
to scan");
```
##########
File path:
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java
##########
@@ -347,7 +385,11 @@ public Options getOptions() {
"make end row exclusive (by default it's inclusive)");
optEndRowExclusive.setArgName("end-exclusive");
scanOptRow = new Option("r", "row", true, "row to scan");
- scanOptColumns = new Option("c", "columns", true, "comma-separated
columns");
+ scanOptColumns = new Option("c", "columns", true,
+ "comma-separated columns.This" + "option is mutually exclusive with
cf");
Review comment:
```suggestion
"comma-separated columns.This" + " option is mutually exclusive with
cf and cq");
```
##########
File path: test/src/main/java/org/apache/accumulo/test/ShellServerIT.java
##########
@@ -1939,6 +1939,22 @@ public void scans() throws Exception {
ts.exec("deletetable -f t");
}
+ @Test
+ public void scansWithColon() throws Exception {
+ ts.exec("createtable twithcolontest");
+ ts.exec("insert row c:f cq value");
+ ts.exec("scan -r row -cf c:f", true, "value");
+ String result = ts.exec("scan -b row -cf c:f -cq cq -e row");
+ assertEquals(2, result.split("\n").length);
+ result = ts.exec("scan -b row -c cf -cf c:f -cq cq -e row", false);
+ assertTrue(result.contains("mutually exclusive"));
+
+ result = ts.exec("scan -b row -cq col1 -e row", false);
+ assertTrue(result.contains("cannot be empty"));
+
Review comment:
```suggestion
ts.exec("scan -b row -cf c:f -cq cq -e row", true, "value");
ts.exec("scan -b row -c cf -cf c:f -cq cq -e row", false, "mutually
exclusive");
ts.exec("scan -b row -cq col1 -e row", false, "cannot be empty");
```
A slight simplification so we don't have to create the results object to
compare against.
##########
File path:
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java
##########
@@ -285,6 +296,33 @@ protected void fetchColumns(final CommandLine cl, final
ScannerBase scanner,
}
}
+ private void fetchColumsWithCFAndCQ(CommandLine cl, Scanner scanner,
ScanInterpreter interpeter) {
+ String cf = "";
+ String cq = "";
+ if (cl.hasOption(scanOptCf.getOpt())) {
+ cf = cl.getOptionValue(scanOptCf.getOpt());
+ }
+ if (cl.hasOption(scanOptCq.getOpt())) {
+ cq = cl.getOptionValue(scanOptCq.getOpt());
+ }
+
+ if (cf.isEmpty() && !cq.isEmpty()) {
+ String formattedString =
+ String.format("Option - %s when used with (%s" + " cannot be empty
)", scanOptCf.getOpt(),
+ scanOptCq.getOpt(), scanOptCq.getOpt());
Review comment:
```suggestion
String.format("Option -%s is required when using -%s.",
scanOptCf.getOpt(),
scanOptCq.getOpt());
```
Some suggestions are wording to make it easier to understand. Also, extra
parameter was removed and fixed spacing.
##########
File path:
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java
##########
@@ -270,6 +271,16 @@ protected ScanInterpreter getInterpreter(final CommandLine
cl, final String tabl
protected void fetchColumns(final CommandLine cl, final ScannerBase scanner,
final ScanInterpreter formatter) throws UnsupportedEncodingException {
+
+ if ((cl.hasOption(scanOptCf.getOpt()) || cl.hasOption(scanOptCq.getOpt()))
+ && cl.hasOption(scanOptColumns.getOpt())) {
+
+ String formattedString =
+ String.format("Options - %s AND (- %s" + " OR - %s are mutually
exclusive )",
+ scanOptColumns.getOpt(), scanOptCf.getOpt(), scanOptCq.getOpt());
Review comment:
```suggestion
String.format("Option -%s is mutually exclusive with options -%s
and -%s.",
scanOptColumns.getOpt(), scanOptCf.getOpt(),
scanOptCq.getOpt());
```
Some suggestions on wording to make it easier to understand and fixed
spacing.
--
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]
