Hive
  1. Hive
  2. HIVE-454

Support escaping of ; in strings in cli

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.3.0
    • Fix Version/s: 0.4.0
    • Component/s: Clients
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      If ; appears in string literals in a query the hive cli is not able to escape them properly.

      1. hive-454.patch
        3 kB
        Edward Capriolo

        Issue Links

          Activity

          Hide
          Edward Capriolo added a comment -

          Ashish,
          I actually am working on a UDF where this became an issue. My .q file looks like this.

          SELECT dboutput(
          'jdbc:derby:../build/test_dboutput_db;create=true','',''
          ,'CREATE TABLE app_info ( kkey VARCHAR(255) NOT NULL, vvalue VARCHAR(255) NOT NULL )') , dboutput('jdbc:derby:../build/test_dboutput_db','','',
          'INSERT INTO app_info (kkey,vvalue) VALUES (?,?)','1','a')
          limit 1;
          

          the ';create=true' section is failing. My nickname is 'Corner Case'

          Show
          Edward Capriolo added a comment - Ashish, I actually am working on a UDF where this became an issue. My .q file looks like this. SELECT dboutput( 'jdbc:derby:../build/test_dboutput_db;create=true','','' ,'CREATE TABLE app_info ( kkey VARCHAR(255) NOT NULL, vvalue VARCHAR(255) NOT NULL )') , dboutput('jdbc:derby:../build/test_dboutput_db','','', 'INSERT INTO app_info (kkey,vvalue) VALUES (?,?)','1','a') limit 1; the ';create=true' section is failing. My nickname is 'Corner Case'
          Hide
          Edward Capriolo added a comment -

          Updated CLIDriver and added test case

          Show
          Edward Capriolo added a comment - Updated CLIDriver and added test case
          Hide
          Edward Capriolo added a comment -

          This patch allows an escaped ; in a string. Removes the escape before passing the query to the driver.

          Show
          Edward Capriolo added a comment - This patch allows an escaped ; in a string. Removes the escape before passing the query to the driver.
          Hide
          Raghotham Murthy added a comment -

          It seems cleaner to modify hive.g instead of adding more parsing/manipulation of query strings in CliDriver.

          Show
          Raghotham Murthy added a comment - It seems cleaner to modify hive.g instead of adding more parsing/manipulation of query strings in CliDriver.
          Hide
          Edward Capriolo added a comment -

          That is a good point. However it seems that the CLIDriver will be have to change regardless. Since the CLIDriver is specifically splitting on ';' a statement like "set a=5; set b=8" gets passed as two separate qp.run()'s.

          Show
          Edward Capriolo added a comment - That is a good point. However it seems that the CLIDriver will be have to change regardless. Since the CLIDriver is specifically splitting on ';' a statement like "set a=5; set b=8" gets passed as two separate qp.run()'s.
          Hide
          Edward Capriolo added a comment -

          Digging into this some more it seems that the query processor and cli driver would have to work together in a different way.

          As of now the CLIDriver is physically splitting on ';' it can not tell the difference between ';' and '/;'. My patch adds some logic to detect '/;' and unescape it before passing to qp.run(String).

          Right now we can argue that the CliDriver has an implicit 'autocommit=true'. We can make this an explicit variable. with autocommit='false' the query processor would hold a StringBuffer or a queue of query strings. 'commit ' ; would cause QP to run each query in its queue.

          This is a non-trivial change. If we add this feature we can set autocommit=true so the default behavior does not change. For no though my patch has a fairly way to deal with this problem.

          Show
          Edward Capriolo added a comment - Digging into this some more it seems that the query processor and cli driver would have to work together in a different way. As of now the CLIDriver is physically splitting on ';' it can not tell the difference between ';' and '/;'. My patch adds some logic to detect '/;' and unescape it before passing to qp.run(String). Right now we can argue that the CliDriver has an implicit 'autocommit=true'. We can make this an explicit variable. with autocommit='false' the query processor would hold a StringBuffer or a queue of query strings. 'commit ' ; would cause QP to run each query in its queue. This is a non-trivial change. If we add this feature we can set autocommit=true so the default behavior does not change. For no though my patch has a fairly way to deal with this problem.
          Hide
          Ashish Thusoo added a comment -

          Ownership = Patch submitter...

          Show
          Ashish Thusoo added a comment - Ownership = Patch submitter...
          Hide
          Namit Jain added a comment -

          The changes look OK to me -

          Raghu, for this to work with Hive.g - the entire string would have to be passed to qp. So, essentially qp will take in a bunch of queries and then parse them instead of getting one query at a time.
          This requires substantial change and should not block https://issues.apache.org/jira/browse/HIVE-645

          Show
          Namit Jain added a comment - The changes look OK to me - Raghu, for this to work with Hive.g - the entire string would have to be passed to qp. So, essentially qp will take in a bunch of queries and then parse them instead of getting one query at a time. This requires substantial change and should not block https://issues.apache.org/jira/browse/HIVE-645
          Hide
          Namit Jain added a comment -

          @Raghu, are you OK with this change - it is OK from my end

          Show
          Namit Jain added a comment - @Raghu, are you OK with this change - it is OK from my end
          Hide
          Namit Jain added a comment -

          Committed. Thanks Edward

          Show
          Namit Jain added a comment - Committed. Thanks Edward

            People

            • Assignee:
              Edward Capriolo
              Reporter:
              Ashish Thusoo
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development