Manno15 commented on a change in pull request #2255:
URL: https://github.com/apache/accumulo/pull/2255#discussion_r708512072
##########
File path:
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java
##########
@@ -59,7 +59,8 @@
public class ScanCommand extends Command {
private Option scanOptAuths, scanOptRow, scanOptColumns,
disablePaginationOpt, showFewOpt,
- formatterOpt, interpreterOpt, formatterInterpeterOpt, outputFileOpt;
+ formatterOpt, interpreterOpt, formatterInterpeterOpt, outputFileOpt,
scanOptCfOptions,
+ scanOptColQualifier;
Review comment:
```suggestion
formatterOpt, interpreterOpt, formatterInterpeterOpt, outputFileOpt,
scanOptColFam,
scanOptColQualifier;
```
Doesn't have to be this specific wording but the two added opts should have
a similar naming convention.
##########
File path:
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java
##########
@@ -348,6 +387,9 @@ public Options getOptions() {
optEndRowExclusive.setArgName("end-exclusive");
scanOptRow = new Option("r", "row", true, "row to scan");
scanOptColumns = new Option("c", "columns", true, "comma-separated
columns");
+ scanOptCfOptions = new Option("cf", "column-family", true, "Column
Family");
+ scanOptColQualifier = new Option("cq", "column-qualifier", true, "Column
Qualifier");
Review comment:
```suggestion
scanOptCfOptions = new Option("cf", "column-family", true, "column
family");
scanOptColQualifier = new Option("cq", "column-qualifier", true, "column
qualifier");
```
To match the rest of the options. Could also add more detail such as `column
family to scan`.
##########
File path:
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java
##########
@@ -285,6 +297,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(scanOptCfOptions.getOpt())) {
+ cf = cl.getOptionValue(scanOptCfOptions.getOpt());
+ }
+ if (cl.hasOption(scanOptColQualifier.getOpt())) {
+ cq = cl.getOptionValue(scanOptColQualifier.getOpt());
+ }
+
+ if (cf.isEmpty() && !cq.isEmpty()) {
+ String formattedString = String.format(
+ "Option - %s when used with (- %s" + "cannot be empty ",
scanOptCfOptions.getOpt(),
Review comment:
Like above, the spacing is also a little off here. This is how it looks
in the shell: ` Option - cf when used with (- cqcannot be empty`
##########
File path: test/src/main/java/org/apache/accumulo/test/ShellServerIT.java
##########
@@ -1939,6 +1939,16 @@ public void scans() throws Exception {
ts.exec("deletetable -f t");
}
+ @Test
+ public void scansWithColon() throws Exception {
+ ts.exec("createtable twithcolontest");
+ make10WithColon();
+ ts.exec("scan -r row0 -cf c:f", true, "value");
+ String result = ts.exec("scan -b row1 -cf c:f -cq col1 -e row1");
+ assertEquals(2, result.split("\n").length);
Review comment:
Could add a test case for if a user adds the `-cq` option without adding
a `-cf` and one where they also add the `-c` option.
##########
File path:
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java
##########
@@ -270,6 +272,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(scanOptCfOptions.getOpt()) ||
cl.hasOption(scanOptColQualifier.getOpt()))
+ && cl.hasOption(scanOptColumns.getOpt())) {
+
+ String formattedString =
+ String.format("Options - %s AND (- %s" + "OR - %s are mutually
exclusive ",
Review comment:
The spacing is a little off here. This is how it looks in the shell:
`Options - c AND (- cfOR - cq are mutually exclusive`
##########
File path: test/src/main/java/org/apache/accumulo/test/ShellServerIT.java
##########
@@ -1939,6 +1939,16 @@ public void scans() throws Exception {
ts.exec("deletetable -f t");
}
+ @Test
+ public void scansWithColon() throws Exception {
+ ts.exec("createtable twithcolontest");
+ make10WithColon();
Review comment:
```suggestion
ts.exec("insert row c:f cq value")
```
This test really only needs one row in the table.
##########
File path:
shell/src/main/java/org/apache/accumulo/shell/commands/QuotedStringTokenizer.java
##########
@@ -72,7 +72,7 @@ private void createTokens() throws BadArgumentException,
UnsupportedEncodingExce
inEscapeSequence = false;
if (ch == 'x') {
hexChars = "";
- } else if (ch == ' ' || ch == '\'' || ch == '"' || ch == '\\') {
+ } else if (ch == ' ' || ch == '\'' || ch == '"' || ch == '\\' || ch ==
':') {
Review comment:
Not sure if this change is necessary for regards to this PR. If you
think it still is then some of the comments should be updated to include the
':' being added.
--
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]
