[ 
https://issues.apache.org/jira/browse/HDFS-12321?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16139316#comment-16139316
 ] 

Chen Liang edited comment on HDFS-12321 at 8/23/17 11:18 PM:
-------------------------------------------------------------

Thanks Anu for the patch, I thought about some of these changes also, but was 
planning for another patch. One thing I found tricky was what's the best way to 
take arguments from command line for the sql queries. For example if I want to 
get count of bucket from volume X, but ignoring volumes starts with Y. So we 
want to take X and Y from command line. I think ideally we specify in json file 
a sql like "select count(\*) as count from bucketInfo where volumeName=X and 
bucketName!=Y" and then take X and Y from commandline. 

Now the issue is, given two argument from command, say M and N, what is the 
best way to assign the arguments, i.e.whether it is X=M,Y=N or the other way. 
Ideally, I think they should be specified with options. Such as writing on 
command line with something like -x M -y N, then we know it is X=M and Y=N. But 
this means json command file must also provide -x and -y options and how to 
parse them. This adds some complexity to the code which I was targeting for a 
separate patch.

Seems v007 is taking arguments in order. Namely, if in sql string X comes 
before Y and in command line M comes before N, then X=M,Y=N, right? I guess I'm 
fine with this. But still, I'm not quite sure about this part:
{code}
        command.getArguments().forEach((String argument) -> {
          if (org.apache.commons.lang3.StringUtils.isNoneBlank(argument)) {
            //TODO check if the Argument already exists, if so don't add it
            // and just use it.
            commandBuilder.withLongOpt(argument);
          }
        });
{code}
I might be totally wrong here. But I as far as I remember, {{withLongOpt}} is 
specifying the longer version name of the command. For example, 
{{x.withLongOpt("createContainer").create("c")}} means you can type either -c 
or -createContainer, they both recognized as same createContainer command. If 
this is correct, then this code snippet is basically keeping overwriting 
withLongOpt with each of the arguments. In the end, the arguments do not seem 
to be correctly taken and passed from command line to command instance. 

Additionally, that change to {{TestPlanner}} seems unnecessary.
 


was (Author: vagarychen):
Thanks Anu for the patch, I thought about some of these changes also, but was 
planning for another patch. One thing I found tricky was what's the best way to 
take arguments from command line for the sql queries. For example if I want to 
get count of bucket from volume X, but ignoring volumes starts with Y. So we 
want to take X and Y from command line. I think ideally we specify in json file 
a sql like "select count(*) as count from bucketInfo where volumeName=X and 
bucketName!=Y" and then take X and Y from commandline. 

Now the issue is, given two argument from command, say M and N, what is the 
best way to assign the arguments, i.e.whether it is X=M,Y=N or the other way. 
Ideally, I think they should be specified with options. Such as writing on 
command line with something like -x M -y N, then we know it is X=M and Y=N. But 
this means json command file must also provide -x and -y options and how to 
parse them. This adds some complexity to the code which I was targeting for a 
separate patch.

Seems v007 is taking arguments in order. Namely, if in sql string X comes 
before Y and in command line M comes before N, then X=M,Y=N, right? I guess I'm 
fine with this. But still, I'm not quite sure about this part:
{code}
        command.getArguments().forEach((String argument) -> {
          if (org.apache.commons.lang3.StringUtils.isNoneBlank(argument)) {
            //TODO check if the Argument already exists, if so don't add it
            // and just use it.
            commandBuilder.withLongOpt(argument);
          }
        });
{code}
I might be totally wrong here. But I as far as I remember, {{withLongOpt}} is 
specifying the longer version name of the command. For example, 
{{x.withLongOpt("createContainer").create("c")}} means you can type either -c 
or -createContainer, they both recognized as same createContainer command. If 
this is correct, then this code snippet is basically keeping overwriting 
withLongOpt with each of the arguments. In the end, the arguments do not seem 
to be correctly taken and passed from command line to command instance. 

Additionally, that change to {{TestPlanner}} seems unnecessary.
 

> Ozone : debug cli: add support to load user-provided SQL query
> --------------------------------------------------------------
>
>                 Key: HDFS-12321
>                 URL: https://issues.apache.org/jira/browse/HDFS-12321
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Chen Liang
>            Assignee: Chen Liang
>             Fix For: ozone
>
>         Attachments: HDFS-12321-HDFS-7240.001.patch, 
> HDFS-12321-HDFS-7240.002.patch, HDFS-12321-HDFS-7240.003.patch, 
> HDFS-12321-HDFS-7240.004.patch, HDFS-12321-HDFS-7240.005.patch, 
> HDFS-12321-HDFS-7240.006.patch, HDFS-12321-HDFS-7240.007.patch
>
>
> This JIRA extends SQL CLI to support loading a user-provided file that 
> includes any sql query the user wants to run on the SQLite db obtained by 
> converting Ozone metadata db.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to