You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Smith (Code Review)" <ge...@cloudera.org> on 2024/09/11 21:00:53 UTC

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21792


Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................

IMPALA-13322: Fix alter on SystemTables

Fixes SystemTable.load to handle incremental loads - such as during
alter - and correctly update the metastore Table so subsequent
operations see the correct version of the table schema.

Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
---
M fe/src/main/java/org/apache/impala/catalog/SystemTable.java
M tests/custom_cluster/test_query_live.py
2 files changed, 39 insertions(+), 11 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/21792/1
-- 
To view, visit http://gerrit.cloudera.org:8080/21792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21792/3//COMMIT_MSG@12
PS3, Line 12: 
> Done
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 23:14:27 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21792/6/be/src/exec/system-table-scanner.cc
File be/src/exec/system-table-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/21792/6/be/src/exec/system-table-scanner.cc@373
PS6, Line 373:       default:
> I think we need this if we want to return NULL:
I guess general question: do we want a default unset value or NULL specifically? There are several other columns where we leave it unmodified rather than returning NULL.

NULL does seem more appropriate in this instance though, so I'll update it.


http://gerrit.cloudera.org:8080/#/c/21792/6/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:

http://gerrit.cloudera.org:8080/#/c/21792/6/tests/custom_cluster/test_query_live.py@231
PS6, Line 231: ''
> Shouldn't this be NULL? What if 'test_alter' is a numeric column?
In this test it's not. But I'll update to NULL. Are you suggesting we should also test other column types?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Sep 2024 15:18:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Jason Fehr, Wenzhe Zhou, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................

IMPALA-13322: Fix alter on SystemTables

Fixes SystemTable.load to handle incremental loads - such as during
alter - and correctly update the metastore Table so subsequent
operations see the correct version of the table schema.

Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
---
M be/src/exec/system-table-scanner.cc
M fe/src/main/java/org/apache/impala/catalog/SystemTable.java
M tests/custom_cluster/test_query_live.py
3 files changed, 57 insertions(+), 12 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Jason Fehr, Wenzhe Zhou, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................

IMPALA-13322: Fix alter on SystemTables

Fixes SystemTable.load to handle incremental loads - such as during
alter - and correctly update the metastore Table so subsequent
operations see the correct version of the table schema.

Updates QueryScanner to allow unknown columns and log a warning. This is
primarily needed when another coordinator has been upgraded and modified
table metadata with new columns that this coordinator doesn't recognize.

Adds TestQueryLive::test_alter to test that altering the table works and
the table is usable afterwards, including attempting to query unknown
columns as we might encounter them during upgrades.

Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
---
M be/src/exec/system-table-scanner.cc
M be/src/exec/system-table-scanner.h
M fe/src/main/java/org/apache/impala/catalog/SystemTable.java
M tests/custom_cluster/test_query_live.py
4 files changed, 76 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/21792/6
-- 
To view, visit http://gerrit.cloudera.org:8080/21792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21792/3/be/src/exec/system-table-scanner.cc
File be/src/exec/system-table-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/21792/3/be/src/exec/system-table-scanner.cc@373
PS3, Line 373:         return Status(Substitute("Unknown column (position $0) added to table",
> In the scenario of two Impala clusters sharing catalogd and statestore, if 
That makes sense to me. I'll update it, probably log a warning.


http://gerrit.cloudera.org:8080/#/c/21792/3/fe/src/main/java/org/apache/impala/catalog/SystemTable.java
File fe/src/main/java/org/apache/impala/catalog/SystemTable.java:

http://gerrit.cloudera.org:8080/#/c/21792/3/fe/src/main/java/org/apache/impala/catalog/SystemTable.java@118
PS3, Line 118:     Table.LOADING_TABLES.decrementAndGet();
> We need a finally-clause for this since parseColumnType() could throw excep
Ok, I was actually thinking more about this last night. For IMPALA-915, we want some way to interrupt Frontend threads, and that may require introducing some new exceptions. So making sure cleanup that needs to happen is in a finally block seems like a good general practice.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 16:38:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16938/ : 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/21792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 18:05:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21792/6/be/src/exec/system-table-scanner.cc
File be/src/exec/system-table-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/21792/6/be/src/exec/system-table-scanner.cc@373
PS6, Line 373:       default:
I think we need this if we want to return NULL:

 tuple->SetNull(slot_desc->null_indicator_offset());


http://gerrit.cloudera.org:8080/#/c/21792/6/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:

http://gerrit.cloudera.org:8080/#/c/21792/6/tests/custom_cluster/test_query_live.py@231
PS6, Line 231: ''
Shouldn't this be NULL? What if 'test_alter' is a numeric column?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Sep 2024 09:16:41 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Sat, 14 Sep 2024 09:57:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16928/ : 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/21792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Sep 2024 21:57:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 4:

(2 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python2 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with virtualenv installed.

http://gerrit.cloudera.org:8080/#/c/21792/4/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:

http://gerrit.cloudera.org:8080/#/c/21792/4/tests/custom_cluster/test_query_live.py@232
PS4, Line 232: +
flake8: W504 line break after binary operator


http://gerrit.cloudera.org:8080/#/c/21792/4/tests/custom_cluster/test_query_live.py@236
PS4, Line 236: )
flake8: E501 line too long (91 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 17:35:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 8: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Sat, 14 Sep 2024 04:46:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Jason Fehr, Wenzhe Zhou, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................

IMPALA-13322: Fix alter on SystemTables

Fixes SystemTable.load to handle incremental loads - such as during
alter - and correctly update the metastore Table so subsequent
operations see the correct version of the table schema.

Updates QueryScanner to allow unknown columns and log a warning. This is
primarily needed when another coordinator has been upgraded and modified
table metadata with new columns that this coordinator doesn't recognize.

Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
---
M be/src/exec/system-table-scanner.cc
M be/src/exec/system-table-scanner.h
M fe/src/main/java/org/apache/impala/catalog/SystemTable.java
M tests/custom_cluster/test_query_live.py
4 files changed, 76 insertions(+), 18 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10949/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Sat, 14 Sep 2024 04:46:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21792/6/be/src/exec/system-table-scanner.cc
File be/src/exec/system-table-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/21792/6/be/src/exec/system-table-scanner.cc@373
PS6, Line 373:       default:
> I guess general question: do we want a default unset value or NULL specific
Done


http://gerrit.cloudera.org:8080/#/c/21792/6/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:

http://gerrit.cloudera.org:8080/#/c/21792/6/tests/custom_cluster/test_query_live.py@231
PS6, Line 231: ''
> In this test it's not. But I'll update to NULL. Are you suggesting we shoul
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Sep 2024 17:13:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 6:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21792/3//COMMIT_MSG@12
PS3, Line 12: 
> Please add info on how this patch was tested.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 20:51:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16948/ : 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/21792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Sep 2024 17:43:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Jason Fehr, Wenzhe Zhou, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................

IMPALA-13322: Fix alter on SystemTables

Fixes SystemTable.load to handle incremental loads - such as during
alter - and correctly update the metastore Table so subsequent
operations see the correct version of the table schema.

Updates QueryScanner to allow unknown columns and log a warning. This is
primarily needed when another coordinator has been upgraded and modified
table metadata with new columns that this coordinator doesn't recognize.

Adds TestQueryLive::test_alter to test that altering the table works and
the table is usable afterwards, including attempting to query unknown
columns as we might encounter them during upgrades.

Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
---
M be/src/exec/system-table-scanner.cc
M be/src/exec/system-table-scanner.h
M fe/src/main/java/org/apache/impala/catalog/SystemTable.java
M tests/custom_cluster/test_query_live.py
4 files changed, 78 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/21792/7
-- 
To view, visit http://gerrit.cloudera.org:8080/21792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 6: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21792/3/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:

http://gerrit.cloudera.org:8080/#/c/21792/3/tests/custom_cluster/test_query_live.py@216
PS3, Line 216:   def test_alter(self):
> That's what happens now, see the latest version.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 23:13:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Jason Fehr, Wenzhe Zhou, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................

IMPALA-13322: Fix alter on SystemTables

Fixes SystemTable.load to handle incremental loads - such as during
alter - and correctly update the metastore Table so subsequent
operations see the correct version of the table schema.

Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
---
M fe/src/main/java/org/apache/impala/catalog/SystemTable.java
M tests/custom_cluster/test_query_live.py
2 files changed, 38 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16926/ : 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/21792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Sep 2024 21:32:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21792/3/be/src/exec/system-table-scanner.cc
File be/src/exec/system-table-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/21792/3/be/src/exec/system-table-scanner.cc@373
PS3, Line 373:         return Status(Substitute("Unknown column (position $0) added to table",
In the scenario of two Impala clusters sharing catalogd and statestore, if they are in different versions which have different columns in the sys.impala_query_live table, this seems the older version cluster can't use the query_live table until it upgrades to the same version. Can we just return NULL for unknown columns?


http://gerrit.cloudera.org:8080/#/c/21792/3/fe/src/main/java/org/apache/impala/catalog/SystemTable.java
File fe/src/main/java/org/apache/impala/catalog/SystemTable.java:

http://gerrit.cloudera.org:8080/#/c/21792/3/fe/src/main/java/org/apache/impala/catalog/SystemTable.java@118
PS3, Line 118:     Table.LOADING_TABLES.decrementAndGet();
We need a finally-clause for this since parseColumnType() could throw exceptions.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 02:09:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Jason Fehr, Wenzhe Zhou, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................

IMPALA-13322: Fix alter on SystemTables

Fixes SystemTable.load to handle incremental loads - such as during
alter - and correctly update the metastore Table so subsequent
operations see the correct version of the table schema.

Updates QueryScanner to allow unknown columns and log a warning. This is
primarily needed when another coordinator has been upgraded and modified
table metadata with new columns that this coordinator doesn't recognize.

Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
---
M be/src/exec/system-table-scanner.cc
M be/src/exec/system-table-scanner.h
M fe/src/main/java/org/apache/impala/catalog/SystemTable.java
M tests/custom_cluster/test_query_live.py
4 files changed, 75 insertions(+), 18 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16939/ : 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/21792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 18:07:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16927/ : 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/21792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Sep 2024 21:33:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 7: Code-Review+2

Carry +1 from Jason


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Sat, 14 Sep 2024 02:03:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

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/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................

IMPALA-13322: Fix alter on SystemTables

Fixes SystemTable.load to handle incremental loads - such as during
alter - and correctly update the metastore Table so subsequent
operations see the correct version of the table schema.

Updates QueryScanner to allow unknown columns and log a warning. This is
primarily needed when another coordinator has been upgraded and modified
table metadata with new columns that this coordinator doesn't recognize.

Adds TestQueryLive::test_alter to test that altering the table works and
the table is usable afterwards, including attempting to query unknown
columns as we might encounter them during upgrades.

Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Reviewed-on: http://gerrit.cloudera.org:8080/21792
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/system-table-scanner.cc
M be/src/exec/system-table-scanner.h
M fe/src/main/java/org/apache/impala/catalog/SystemTable.java
M tests/custom_cluster/test_query_live.py
4 files changed, 78 insertions(+), 18 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 9
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21792/3/be/src/exec/system-table-scanner.cc
File be/src/exec/system-table-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/21792/3/be/src/exec/system-table-scanner.cc@373
PS3, Line 373:       default:
> That makes sense to me. I'll update it, probably log a warning.
Done


http://gerrit.cloudera.org:8080/#/c/21792/3/fe/src/main/java/org/apache/impala/catalog/SystemTable.java
File fe/src/main/java/org/apache/impala/catalog/SystemTable.java:

http://gerrit.cloudera.org:8080/#/c/21792/3/fe/src/main/java/org/apache/impala/catalog/SystemTable.java@118
PS3, Line 118:       refreshLastUsedTime();
> Ok, I was actually thinking more about this last night. For IMPALA-915, we 
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 17:35:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 3:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/21792/3//COMMIT_MSG@12
PS3, Line 12: 
Please add info on how this patch was tested.


http://gerrit.cloudera.org:8080/#/c/21792/3/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:

http://gerrit.cloudera.org:8080/#/c/21792/3/tests/custom_cluster/test_query_live.py@216
PS3, Line 216:   def test_alter(self):
I would prefer if this entire test was wrapped in a try..finally code block where the finally either executes alter table sys.impala_query_live drop column test_alter or drops the entire sys.impala_query_live table to ensure this table is not left in an invalid state.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 20:47:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13322: Fix alter on SystemTables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21792 )

Change subject: IMPALA-13322: Fix alter on SystemTables
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21792/3/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:

http://gerrit.cloudera.org:8080/#/c/21792/3/tests/custom_cluster/test_query_live.py@216
PS3, Line 216:   def test_alter(self):
> I would prefer if this entire test was wrapped in a try..finally code block
That's what happens now, see the latest version.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a59e58c086e659941e0db8a2b893ac6dcc5143a
Gerrit-Change-Number: 21792
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 20:49:42 +0000
Gerrit-HasComments: Yes