[Impala-ASF-CR] IMPALA-4168: Adds Oracle-style hint placement for INSERT/UPSERT

[Kim Jin Chul (Code Review)]

Kim Jin Chul has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8676 )

Change subject: IMPALA-4168: Adds Oracle-style hint placement for INSERT/UPSERT
..


Patch Set 9:

Tho examples for INSERT/UPSERT are added to the commit message.

 > Can you also an example of two of the new allowed syntax? Thanks


--
To view, visit http://gerrit.cloudera.org:8080/8676
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied7629d70197a0270cdc0853e00cc021fdb4dc20
Gerrit-Change-Number: 8676
Gerrit-PatchSet: 9
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Sun, 07 Jan 2018 07:41:39 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3651: Adds murmur hash() built-in function

[Kim Jin Chul (Code Review)]

Hello Attila Jeges, Tim Armstrong, Alex Behm, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8893

to look at the new patch set (#5).

Change subject: IMPALA-3651: Adds murmur_hash() built-in function
..

IMPALA-3651: Adds murmur_hash() built-in function

murmur_hash relys on HashUtil::MurmurHash2_64 which MurmurHash2 64-bit
version.

Testing:
Add unit tests for primitive types: ExprTest.MurmurHashFunction
Add E2E tests into exprs.test

Change-Id: I14d56ffb8fab256f3f66a2669271fd4b3c50cc29
---
M be/src/exprs/expr-test.cc
M be/src/exprs/utility-functions-ir.cc
M be/src/exprs/utility-functions.h
M be/src/util/hash-util.h
M common/function-registry/impala_functions.py
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
6 files changed, 134 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/8893/5
--
To view, visit http://gerrit.cloudera.org:8080/8893
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I14d56ffb8fab256f3f66a2669271fd4b3c50cc29
Gerrit-Change-Number: 8893
Gerrit-PatchSet: 5
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-3651: Adds murmur hash() built-in function

[Kim Jin Chul (Code Review)]

Kim Jin Chul has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8893 )

Change subject: IMPALA-3651: Adds murmur_hash() built-in function
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8893/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:

http://gerrit.cloudera.org:8080/#/c/8893/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test@2922
PS4, Line 2922: select murmur_hash(NULL)
> Tests like this belong in expr-test.cc
Done, thanks for your advice.



--
To view, visit http://gerrit.cloudera.org:8080/8893
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I14d56ffb8fab256f3f66a2669271fd4b3c50cc29
Gerrit-Change-Number: 8893
Gerrit-PatchSet: 4
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sun, 07 Jan 2018 07:11:39 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3282: Adds regexp escape built-in function

[Kim Jin Chul (Code Review)]

Hello Tianyi Wang, Jim Apple, Tim Armstrong,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8900

to look at the new patch set (#4).

Change subject: IMPALA-3282: Adds regexp_escape built-in function
..

IMPALA-3282: Adds regexp_escape built-in function

Escapes the following special characters in RE2 library:
.\+*?[^]$(){}=!<>|:-

Testing:
Add some unit tests into ExprTest.StringRegexpFunctions
Add some E2E tests into exprs.test

Change-Id: I84c3e0ded26f6eb20794c38b75be9b25cd111e4b
---
M be/src/exprs/expr-test.cc
M be/src/exprs/string-functions-ir.cc
M be/src/exprs/string-functions.h
M common/function-registry/impala_functions.py
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
5 files changed, 86 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/8900/4
--
To view, visit http://gerrit.cloudera.org:8080/8900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84c3e0ded26f6eb20794c38b75be9b25cd111e4b
Gerrit-Change-Number: 8900
Gerrit-PatchSet: 4
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6348: Redact only sensitive fields in runtime profiles

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8934 )

Change subject: IMPALA-6348: Redact only sensitive fields in runtime profiles
..


Patch Set 4: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/8934
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3b6726009bf458a7ec73131e5d659b12ab73cf
Gerrit-Change-Number: 8934
Gerrit-PatchSet: 4
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Sat, 06 Jan 2018 22:54:15 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6348: Redact only sensitive fields in runtime profiles

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8934 )

Change subject: IMPALA-6348: Redact only sensitive fields in runtime profiles
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1683/


--
To view, visit http://gerrit.cloudera.org:8080/8934
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3b6726009bf458a7ec73131e5d659b12ab73cf
Gerrit-Change-Number: 8934
Gerrit-PatchSet: 4
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Sat, 06 Jan 2018 19:19:10 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" command

[Adam Holley (Code Review)]

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8928 )

Change subject: IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" 
command
..


Patch Set 3:

(10 comments)

Applied changes as recommended.

http://gerrit.cloudera.org:8080/#/c/8928/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8928/3//COMMIT_MSG@10
PS3, Line 10: metadatad.
> typo: metadata
Done


http://gerrit.cloudera.org:8080/#/c/8928/3/common/thrift/JniCatalog.thrift
File common/thrift/JniCatalog.thrift:

http://gerrit.cloudera.org:8080/#/c/8928/3/common/thrift/JniCatalog.thrift@269
PS3, Line 269: file
> Did you mean 'row' here?
Done


http://gerrit.cloudera.org:8080/#/c/8928/3/fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
File 
fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java:

http://gerrit.cloudera.org:8080/#/c/8928/3/fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java@37
PS3, Line 37: this.
> nit: you can remove 'this'. All the private fields are ending with '_' (see
Done


http://gerrit.cloudera.org:8080/#/c/8928/3/fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java@49
PS3, Line 49: rowFormatParams.setPartition_set(getPartitionSet().toThrift());
:   }
> nit: indentation off by 2
Done


http://gerrit.cloudera.org:8080/#/c/8928/3/fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java@59
PS3, Line 59: tbl instanceof KuduTable
> I think there are a few more cases to consider here. How about HBase tables
Done


http://gerrit.cloudera.org:8080/#/c/8928/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/8928/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@463
PS3, Line 463: if (rowFormatParams.isSetPartition_set()) {
 :   resultColVal.setString_val(
 :   "Updated " + numUpdatedPartitions.getRef() + " 
partition(s).");
 :   } else {
 :   resultColVal.setString_val("Updated table.");
 :   }
> nit: indentation is off
Done


http://gerrit.cloudera.org:8080/#/c/8928/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2269
PS3, Line 2269: boolean
> Add comment about the return value. Also, fix indentation of this function.
Done


http://gerrit.cloudera.org:8080/#/c/8928/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2281
PS3, Line 2281: if (rowFormat.getFieldDelimiter() != null) {
  : sd.getSerdeInfo().putToParameters(
  : "serialization.format", 
rowFormat.getFieldDelimiter());
  : sd.getSerdeInfo().putToParameters("field.delim",
  : rowFormat.getFieldDelimiter());
  : }
  : if (rowFormat.getEscapeChar() != null) {
  : sd.getSerdeInfo().putToParameters("escape.delim", 
rowFormat.getEscapeChar());
  : }
  : if (rowFormat.getLineDelimiter() != null) {
  : sd.getSerdeInfo().putToParameters("line.delim", 
rowFormat.getLineDelimiter());
  : }
> That block is similar to the block between L2305-2318. I believe you should
Done


http://gerrit.cloudera.org:8080/#/c/8928/3/tests/query_test/test_delimited_text.py
File tests/query_test/test_delimited_text.py:

http://gerrit.cloudera.org:8080/#/c/8928/3/tests/query_test/test_delimited_text.py@73
PS3, Line 73: def test_delimited_text_alter(self, vector, unique_database):
> I believe most of these tests should be in test_ddl.py (alter-table.test).
Done


http://gerrit.cloudera.org:8080/#/c/8928/3/tests/query_test/test_delimited_text.py@148
PS3, Line 148: def test_delimited_text_alter_kudu_fail(self, vector, 
unique_database):
 : """ Test alter row format statement.  IMPALA-4323.  Ensure 
alter statement fails
 : for Kudu. """
 : self.execute_query_expect_success(self.client, """
 :   create table if not exists %s.kdelimit
 :   (c1 int primary key, c2 string, c3 string)
 :   stored as kudu
 :   """ % unique_database)
 : # Try to change the field delimiter
 : ex = self.execute_query_expect_failure(self.client, """
 :   alter table %s.kdelimit
 :   set row format delimited
 :   fields terminated by ' '
 :   """ % unique_database)
 : assert "ALTER TABLE SET ROW FORMAT is not supported on Kudu" 
in str(ex)
> That should be an analysis test, along with any other test cases that exerc
Done

[Impala-ASF-CR] IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" command

[Adam Holley (Code Review)]

Adam Holley has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/8928 )

Change subject: IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" 
command
..

IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" command

Examples of new command:
ALTER TABLE t1 SET ROW FORMAT DELIMITED FIELDS TERMINATED BY '\002';
ALTER TABLE t1 SET ROW FORMAT DELIMITED LINES TERMINATED BY '\001';

Testing:
Added parser tests and unit tests for alter statements including
partition options.

Change-Id: I96e347463504915a6f33932552e4d1f61e9b1154
---
M common/thrift/JniCatalog.thrift
M fe/src/main/cup/sql-parser.cup
A fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/catalog/HiveStorageDescriptorFactory.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/workloads/functional-query/queries/QueryTest/alter-table.test
8 files changed, 260 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/8928/4
--
To view, visit http://gerrit.cloudera.org:8080/8928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I96e347463504915a6f33932552e4d1f61e9b1154
Gerrit-Change-Number: 8928
Gerrit-PatchSet: 4
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kim Jin Chul 

[Impala-ASF-CR] IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" command

[Adam Holley (Code Review)]

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8928 )

Change subject: IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" 
command
..


Patch Set 1:

(4 comments)

Updated patch 1.

http://gerrit.cloudera.org:8080/#/c/8928/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8928/1//COMMIT_MSG@7
PS1, Line 7: Added the "SET ROW FORMAT" option to the "ALTER TABLE" command.
> Adds "IMPALA-4323: " at the start of the line.
Done


http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:

http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/cup/sql-parser.cup@1003
PS1, Line 1003:   | KW_ALTER KW_TABLE table_name:table 
opt_partition_set:partition KW_SET
> Add some unit tests(white & black) into ParserTest.java. See
Done


http://gerrit.cloudera.org:8080/#/c/8928/3/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:

http://gerrit.cloudera.org:8080/#/c/8928/3/fe/src/main/cup/sql-parser.cup@1007
PS3, Line 1007: // row_format_val cannot be used as it conflicts with 
cache_op_val.
> Hm, why would it conflict with cache_op_val? Can you post the error message
When I dump the grammar, the following two lines seem to be the problem:
[128] alter_tbl_stmt ::= KW_ALTER KW_TABLE table_name opt_partition_set KW_SET 
row_format_val escaped_by_val line_terminator_val
[129] alter_tbl_stmt ::= KW_ALTER KW_TABLE table_name opt_partition_set KW_SET 
cache_op_val
Here are the errors:
[WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** 
Reduce/Reduce conflict found in state #1242
  between row_format_val ::= (*)
  and cache_op_val ::= (*)
  under symbols: {EOF, SEMICOLON}
  Resolved in favor of the second production.

[WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** 
Shift/Reduce conflict found in state #1242
  between row_format_val ::= (*)
  under symbol EOF
  Resolved in favor of shifting.

[WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** 
Shift/Reduce conflict found in state #1242
  between row_format_val ::= (*)
  under symbol SEMICOLON
  Resolved in favor of shifting.

[WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** 
Shift/Reduce conflict found in state #1242
  between cache_op_val ::= (*)
  under symbol EOF
  Resolved in favor of shifting.

[WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** 
Shift/Reduce conflict found in state #1242
  between cache_op_val ::= (*)
  under symbol SEMICOLON
  Resolved in favor of shifting.

[WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** 
Shift/Reduce conflict found in state #892
  between line_terminator_val ::= (*)
  and line_terminator_val ::= (*) KW_LINES terminator_val
  under symbol KW_LINES
  Resolved in favor of shifting.

[WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** 
Shift/Reduce conflict found in state #890
  between escaped_by_val ::= (*)
  and escaped_by_val ::= (*) KW_ESCAPED KW_BY STRING_LITERAL
  under symbol KW_ESCAPED
  Resolved in favor of shifting.


http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
File 
fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java:

http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java@35
PS1, Line 35:
> nit: use space instead of tab
Done



--
To view, visit http://gerrit.cloudera.org:8080/8928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96e347463504915a6f33932552e4d1f61e9b1154
Gerrit-Change-Number: 8928
Gerrit-PatchSet: 1
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Sat, 06 Jan 2018 17:51:16 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" command

[Adam Holley (Code Review)]

Adam Holley has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/8928 )

Change subject: IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" 
command
..

IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" command

Updated the sql parser and added code to properly update the
metadatad.

Testing:
Added parser tests and unit tests for alter statements including
partition options.

Change-Id: I96e347463504915a6f33932552e4d1f61e9b1154
---
M common/thrift/JniCatalog.thrift
M fe/src/main/cup/sql-parser.cup
A fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M tests/query_test/test_delimited_text.py
6 files changed, 317 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/8928/2
--
To view, visit http://gerrit.cloudera.org:8080/8928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I96e347463504915a6f33932552e4d1f61e9b1154
Gerrit-Change-Number: 8928
Gerrit-PatchSet: 2
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kim Jin Chul 

[Impala-ASF-CR] IMPALA-3282: Adds regexp escape built-in function

[Kim Jin Chul (Code Review)]

Kim Jin Chul has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8900 )

Change subject: IMPALA-3282: Adds regexp_escape built-in function
..


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/8900/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8900/2//COMMIT_MSG@10
PS2, Line 10: ".*\\+?^[](){}$!=:-#\n\r\t\v "
> Where does this list come from? Impala uses RE2 syntax, which does not esca
I've collected the special characters again.


http://gerrit.cloudera.org:8080/#/c/8900/2/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8900/2/be/src/exprs/expr-test.cc@4157
PS2, Line 4157:   TestStringValue("regexp_escape('Hello.world')", 
"Hello\\.world");
> I think the examples in this file could be easier for a reader to understan
Done. Thanks for the information.


http://gerrit.cloudera.org:8080/#/c/8900/2/be/src/exprs/expr-test.cc@4159
PS2, Line 4159:   TestStringValue("regexp_escape('Helloworld')", 
"Helloworld");
> It seems that the parameter to the regexp_escape function is escaped once s
Done. I've added a comment.


http://gerrit.cloudera.org:8080/#/c/8900/2/be/src/exprs/expr-test.cc@4185
PS2, Line 4185:   
TestStringValue("regexp_escape('a.b*cd+e?f^g[h]i(j)k{l}m$n!o=p:q-r#s\nt\ru\tv\vw
 x"
> We should also directly test that the escaping is correct for our other reg
Done. I've added some mixed case with other regexp_*.


http://gerrit.cloudera.org:8080/#/c/8900/2/be/src/exprs/string-functions-ir.cc
File be/src/exprs/string-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/8900/2/be/src/exprs/string-functions-ir.cc@624
PS2, Line 624:   const string input = AnyValUtil::ToString(str);
> We can directly iterate with the pointer here. e.g. for (char* c = str.ptr;
Done


http://gerrit.cloudera.org:8080/#/c/8900/2/be/src/exprs/string-functions-ir.cc@627
PS2, Line 627: const bool need_escape = special_character_set.find(c) != 
special_character_set.end();
> I think using std::find on the string literal might be faster than on a set
Done


http://gerrit.cloudera.org:8080/#/c/8900/2/be/src/exprs/string-functions-ir.cc@638
PS2, Line 638:   default: ss << "\\" << c; break;
> Use '\\'.
Done



--
To view, visit http://gerrit.cloudera.org:8080/8900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84c3e0ded26f6eb20794c38b75be9b25cd111e4b
Gerrit-Change-Number: 8900
Gerrit-PatchSet: 2
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 06 Jan 2018 13:10:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3282: Adds regexp escape built-in function

[Kim Jin Chul (Code Review)]

Hello Tianyi Wang, Jim Apple, Tim Armstrong,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8900

to look at the new patch set (#3).

Change subject: IMPALA-3282: Adds regexp_escape built-in function
..

IMPALA-3282: Adds regexp_escape built-in function

Escapes the following special characters:
.\+*?[^]$(){}=!<>|:-

Testing:
Add some unit tests into ExprTest.StringRegexpFunctions
Add some E2E tests into exprs.test

Change-Id: I84c3e0ded26f6eb20794c38b75be9b25cd111e4b
---
M be/src/exprs/expr-test.cc
M be/src/exprs/string-functions-ir.cc
M be/src/exprs/string-functions.h
M common/function-registry/impala_functions.py
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
5 files changed, 81 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/8900/3
--
To view, visit http://gerrit.cloudera.org:8080/8900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84c3e0ded26f6eb20794c38b75be9b25cd111e4b
Gerrit-Change-Number: 8900
Gerrit-PatchSet: 3
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

[Zoltan Borok-Nagy (Code Review)]

Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8490 )

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..


Patch Set 22:

Thank you all for the comments!


--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 22
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Sat, 06 Jan 2018 10:00:26 +
Gerrit-HasComments: No