You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Wood (Code Review)" <ge...@cloudera.org> on 2017/09/28 17:46:17 UTC
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Tim Wood has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8166
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
IMPALA-5986: Correct set-option logic to recognize digits in names.
Arose during work for IMPALA-5376; prevents tests from passing consistently.
Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
---
M tests/common/impala_test_suite.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/66/8166/1
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8166 )
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 Sep 2017 01:50:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8166 )
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
I'm fine with this as is, since it's strictly better, but I think it may make sense to clear up the [A-Z] stuff if it's truly case-insensitive already.
http://gerrit.cloudera.org:8080/#/c/8166/1/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/8166/1/tests/common/impala_test_suite.py@103
PS1, Line 103: SET_PATTERN = re.compile(r'\s*set\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*=*', re.I)
Since this is a case insensitive RE (re.I), I think you don't need the A-Z here.
If we wanted to be extra careful, we could write a test because Thrift exposes the metadata for us like so:
>>> import ImpalaInternalService.ttypes
>>> options = [ x[2] for x in ImpalaInternalService.ttypes.TQueryOptions.thrift_spec if x ]
>>> options
['abort_on_error', 'max_errors', 'disable_codegen', 'batch_size', 'num_nodes', 'max_scan_range_length', 'num_scanner_threads', 'max_io_buffers', 'allow_unsupported_formats', 'default_order_by_limit', 'debug_action', 'mem_limit', 'abort_on_default_limit_exceeded', 'compression_codec', 'hbase_caching', 'hbase_cache_blocks', 'parquet_file_size', 'explain_level', 'sync_ddl', 'request_pool', 'v_cpu_cores', 'reservation_request_timeout', 'disable_cached_reads', 'disable_outermost_topn', 'rm_initial_mem', 'query_timeout_s', 'buffer_pool_limit', 'appx_count_distinct', 'disable_unsafe_spills', 'seq_compression_mode', 'exec_single_node_rows_threshold', 'optimize_partition_key_scans', 'replica_preference', 'schedule_random_replica', 'scan_node_codegen_threshold', 'disable_streaming_preaggregations', 'runtime_filter_mode', 'runtime_bloom_filter_size', 'runtime_filter_wait_time_ms', 'disable_row_runtime_filtering', 'max_num_runtime_filters', 'parquet_annotate_strings_utf8', 'parquet_fallback_schema_resolution', 'mt_dop', 's3_skip_insert_staging', 'runtime_filter_min_size', 'runtime_filter_max_size', 'prefetch_mode', 'strict_mode', 'scratch_limit', 'enable_expr_rewrites', 'decimal_v2', 'parquet_dictionary_filtering', 'parquet_array_resolution', 'parquet_read_statistics', 'default_join_distribution_mode', 'disable_codegen_rows_threshold', 'default_spillable_buffer_size', 'min_spillable_buffer_size', 'max_row_size']
That said, I think the RE now matches what a Thrift identifier per https://thrift.apache.org/docs/idl can be. The docs also say "." can happen, but I don't think you can have a field with a dot in it.
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2017 17:58:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/8166 )
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
Patch Set 1: Code-Review+2
Did this fix your flapping problem?
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2017 17:57:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8166 )
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1275/
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2017 21:47:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Tim Wood (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, Philip Zeyliger, David Knupp, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8166
to look at the new patch set (#2).
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
IMPALA-5986: Correct set-option logic to recognize digits in names.
Arose during work for IMPALA-5376; prevents tests from passing consistently.
Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
---
M tests/common/impala_test_suite.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/66/8166/2
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Tim Wood (Code Review)" <ge...@cloudera.org>.
Tim Wood has posted comments on this change. ( http://gerrit.cloudera.org:8080/8166 )
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8166/1/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/8166/1/tests/common/impala_test_suite.py@103
PS1, Line 103: SET_PATTERN = re.compile(r'\s*set\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*=*', re.I)
> I don't think it matters much, so I'll leave it to your discretion. We can
Done
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2017 21:45:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8166 )
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1274/
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2017 21:37:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Tim Wood (Code Review)" <ge...@cloudera.org>.
Tim Wood has posted comments on this change. ( http://gerrit.cloudera.org:8080/8166 )
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
Patch Set 1:
PS This resolves the query flapping problems observed in earlier runs of change #8102.
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2017 21:34:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Tim Wood (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, Philip Zeyliger, David Knupp, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8166
to look at the new patch set (#3).
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
IMPALA-5986: Correct set-option logic to recognize digits in names.
Arose during work for IMPALA-5376; prevents tests from passing consistently.
Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
---
M tests/common/impala_test_suite.py
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/66/8166/3
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8166 )
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
Patch Set 1: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1274/
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2017 21:38:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/8166 )
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2017 21:46:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8166 )
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
IMPALA-5986: Correct set-option logic to recognize digits in names.
Arose during work for IMPALA-5376; prevents tests from passing consistently.
Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Reviewed-on: http://gerrit.cloudera.org:8080/8166
Reviewed-by: Michael Brown <mi...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M tests/common/impala_test_suite.py
1 file changed, 2 insertions(+), 2 deletions(-)
Approvals:
Michael Brown: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8166 )
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8166/1/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/8166/1/tests/common/impala_test_suite.py@103
PS1, Line 103: SET_PATTERN = re.compile(r'\s*set\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*=*', re.I)
> This is a slightly ticklish philo. question for me. I'm not opposed to rem
I don't think it matters much, so I'll leave it to your discretion. We can update the comment above it to point out that it matches the IDL. Specifically "alphabets and underscores" is no longer true.
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2017 19:15:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5986: Correct set-option logic to recognize digits in names.
Posted by "Tim Wood (Code Review)" <ge...@cloudera.org>.
Tim Wood has posted comments on this change. ( http://gerrit.cloudera.org:8080/8166 )
Change subject: IMPALA-5986: Correct set-option logic to recognize digits in names.
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8166/1/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/8166/1/tests/common/impala_test_suite.py@103
PS1, Line 103: SET_PATTERN = re.compile(r'\s*set\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*=*', re.I)
> Since this is a case insensitive RE (re.I), I think you don't need the A-Z
This is a slightly ticklish philo. question for me. I'm not opposed to removing A-Z, but this pattern (which does seem to match the Thrift grammar for Identifier) is so iconic to people by now, that the missing A-Z might give one pause before seeing the re.I. This way, the pattern can be lifted into a context that doesn't use (or have) ignore-case matching (like bash, apparently). We still need the re.I for the SET keyword.
--
To view, visit http://gerrit.cloudera.org:8080/8166
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3ba641553ff827dbd4673b9fe7ed7d9d5e83052
Gerrit-Change-Number: 8166
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wood <tw...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Wood <tw...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2017 18:48:25 +0000
Gerrit-HasComments: Yes