You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fredy Wijaya (Code Review)" <ge...@cloudera.org> on 2019/07/02 16:51:42 UTC
[Impala-ASF-CR] IMPALA-8734: Reload table schema on TBLPROPERTIES change
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13785
Change subject: IMPALA-8734: Reload table schema on TBLPROPERTIES change
......................................................................
IMPALA-8734: Reload table schema on TBLPROPERTIES change
Prior to this patch, an INVALIDATE METADATA was required when altering
the TBLPROPERTIES for the changes to take effect. With this patch the
table schema is automatically reloaded on TBLPROPERTIES change.
Testing:
- Added a new test in test_ddl.py
- Ran test_ddl.py
Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/metadata/test_ddl.py
2 files changed, 25 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/13785/3
--
To view, visit http://gerrit.cloudera.org:8080/13785
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
Gerrit-Change-Number: 13785
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
[Impala-ASF-CR] IMPALA-8734: Reload table schema on TBLPROPERTIES change
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13785 )
Change subject: IMPALA-8734: Reload table schema on TBLPROPERTIES change
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4575/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/13785
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
Gerrit-Change-Number: 13785
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fr...@apache.org>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fr...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 20:13:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8734: Reload table schema on TBLPROPERTIES change
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13785 )
Change subject: IMPALA-8734: Reload table schema on TBLPROPERTIES change
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3803/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/13785
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
Gerrit-Change-Number: 13785
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 17:32:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8734: Reload table schema on TBLPROPERTIES change
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13785 )
Change subject: IMPALA-8734: Reload table schema on TBLPROPERTIES change
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3806/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/13785
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
Gerrit-Change-Number: 13785
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fr...@apache.org>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fr...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 20:31:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8734: Reload table schema on TBLPROPERTIES change
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13785 )
Change subject: IMPALA-8734: Reload table schema on TBLPROPERTIES change
......................................................................
Patch Set 3:
(3 comments)
I wrote some comments for the test, but probably the simplest way would be to move the whole logic to alter_table.test. Some strings could be inserted to the table, check select *, then serialization.null.format could be set to one of the values, then check select * again.
http://gerrit.cloudera.org:8080/#/c/13785/3/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:
http://gerrit.cloudera.org:8080/#/c/13785/3/tests/metadata/test_ddl.py@697
PS3, Line 697: # IMPALA-8734: Force a table schema reload when setting table properties.
Is there a reason for not creating a new test? The new code seems to be completely independent from the earlier things in test_create_alter_tbl_properties
http://gerrit.cloudera.org:8080/#/c/13785/3/tests/metadata/test_ddl.py@702
PS3, Line 702: null
Using a less "null like" value could make the test's intention clearer - it was not obvious for me that "null" is not interpreted as NULL by default.
http://gerrit.cloudera.org:8080/#/c/13785/3/tests/metadata/test_ddl.py@701
PS3, Line 701: temp_file, temp_path = tempfile.mkstemp()
: os.write(temp_file, "\nnull\n")
: os.close(temp_file)
: subprocess.check_call(["hdfs", "dfs", "-put", "-f", temp_path,
: "/test-warehouse/{0}.db/{1}/f".format(unique_database,
: tbl_name)])
: self.execute_query_expect_success(self.client,
: "alter table {0}.{1} set tblproperties"
: "('serialization.null.format'='null')"
: .format(unique_database, tbl_name))
First I thought that it would be nice to create a utility function for this, then I realized that there is already one: https://github.com/apache/impala/blob/aee0abd76b762e57ce9f0a2e40a9a8b4f97dc986/tests/util/hdfs_util.py#L172
--
To view, visit http://gerrit.cloudera.org:8080/13785
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
Gerrit-Change-Number: 13785
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 17:50:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8734: Reload table schema on TBLPROPERTIES change
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13785 )
Change subject: IMPALA-8734: Reload table schema on TBLPROPERTIES change
......................................................................
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13785/3/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:
http://gerrit.cloudera.org:8080/#/c/13785/3/tests/metadata/test_ddl.py@697
PS3, Line 697: self.execute_query_expect_success(self.client, "create table {0}.{1} (c1 string)"
> Is there a reason for not creating a new test? The new code seems to be com
No reason, I thought it would be simpler, but yeah it seems creating a new test is better. Done.
http://gerrit.cloudera.org:8080/#/c/13785/3/tests/metadata/test_ddl.py@702
PS3, Line 702: ct_s
> Using a less "null like" value could make the test's intention clearer - it
I was following the test case, but yeah using a different value is more readable. Done.
http://gerrit.cloudera.org:8080/#/c/13785/3/tests/metadata/test_ddl.py@701
PS3, Line 701: file_data="\nfoo\n")
: self.execute_query_expect_success(self.client,
: "alter table {0}.{1} set tblproperties"
: "('serialization.null.format'='foo')"
: .format(unique_database, tbl_name))
: result = self.execute_query_expect_success(self.client,
: "select * from {0}.{1}"
: .format(unique_database, tbl_name))
: assert len(result.data) == 2
: assert result.data[0] == ''
> First I thought that it would be nice to create a utility function for this
TIL. Done.
--
To view, visit http://gerrit.cloudera.org:8080/13785
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
Gerrit-Change-Number: 13785
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 19:52:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8734: Reload table schema on TBLPROPERTIES change
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/13785 )
Change subject: IMPALA-8734: Reload table schema on TBLPROPERTIES change
......................................................................
IMPALA-8734: Reload table schema on TBLPROPERTIES change
Prior to this patch, an INVALIDATE METADATA was required when altering
the TBLPROPERTIES for the changes to take effect. With this patch the
table schema is automatically reloaded on TBLPROPERTIES change.
Testing:
- Added a new test in test_ddl.py
- Ran test_ddl.py
Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/metadata/test_ddl.py
2 files changed, 20 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/13785/4
--
To view, visit http://gerrit.cloudera.org:8080/13785
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
Gerrit-Change-Number: 13785
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-8734: Reload table schema on TBLPROPERTIES change
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13785 )
Change subject: IMPALA-8734: Reload table schema on TBLPROPERTIES change
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13785
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
Gerrit-Change-Number: 13785
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fr...@apache.org>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fr...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 20:13:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8734: Reload table schema on TBLPROPERTIES change
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13785 )
Change subject: IMPALA-8734: Reload table schema on TBLPROPERTIES change
......................................................................
Patch Set 3: Code-Review+1
Looks good to me
--
To view, visit http://gerrit.cloudera.org:8080/13785
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
Gerrit-Change-Number: 13785
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 17:50:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8734: Reload table schema on TBLPROPERTIES change
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/13785 )
Change subject: IMPALA-8734: Reload table schema on TBLPROPERTIES change
......................................................................
IMPALA-8734: Reload table schema on TBLPROPERTIES change
Prior to this patch, an INVALIDATE METADATA was required when altering
the TBLPROPERTIES for the changes to take effect. With this patch the
table schema is automatically reloaded on TBLPROPERTIES change.
Testing:
- Added a new test in test_ddl.py
- Ran test_ddl.py
Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
Reviewed-on: http://gerrit.cloudera.org:8080/13785
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/metadata/test_ddl.py
2 files changed, 20 insertions(+), 0 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/13785
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
Gerrit-Change-Number: 13785
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fr...@apache.org>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fr...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-8734: Reload table schema on TBLPROPERTIES change
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13785 )
Change subject: IMPALA-8734: Reload table schema on TBLPROPERTIES change
......................................................................
Patch Set 4: Code-Review+2
patch looks good to me.
--
To view, visit http://gerrit.cloudera.org:8080/13785
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
Gerrit-Change-Number: 13785
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fr...@apache.org>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fr...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 20:11:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8734: Reload table schema on TBLPROPERTIES change
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13785 )
Change subject: IMPALA-8734: Reload table schema on TBLPROPERTIES change
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13785
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a43a962c2a456f3ddc078b2924f551fccb5c2ad
Gerrit-Change-Number: 13785
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fr...@apache.org>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fr...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 Jul 2019 01:57:03 +0000
Gerrit-HasComments: No