[Impala-ASF-CR] IMPALA-12463: Batch non-consecutive table events in the event processor

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20533 )

Change subject: IMPALA-12463: Batch non-consecutive table events in the event 
processor
..


Patch Set 8:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I849c0306bc46080ee4059854f42d9c217a89b905
Gerrit-Change-Number: 20533
Gerrit-PatchSet: 8
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 08:01:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12463: Batch non-consecutive table events in the event processor

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20533 )

Change subject: IMPALA-12463: Batch non-consecutive table events in the event 
processor
..


Patch Set 8:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I849c0306bc46080ee4059854f42d9c217a89b905
Gerrit-Change-Number: 20533
Gerrit-PatchSet: 8
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 08:05:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-11805: Use llvm ObjectCache for codegen caching

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20733 )

Change subject: IMPALA-11805: Use llvm ObjectCache for codegen caching
..


Patch Set 8:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3c1b46bb9018ed0320817141785a3bdc41fa677
Gerrit-Change-Number: 20733
Gerrit-PatchSet: 8
Gerrit-Owner: Yida Wu 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Mon, 18 Dec 2023 08:16:18 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 08:18:13 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 5:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/krpc-data-stream-recvr.cc
File be/src/runtime/krpc-data-stream-recvr.cc:

http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/krpc-data-stream-recvr.cc@343
PS5, Line 343:   if (payload->rpc_context != nullptr) 
TRACE_TO(payload->rpc_context->trace(), "Enqueuing deferred RPC");
line too long (105 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/krpc-data-stream-recvr.cc@493
PS5, Line 493: //TRACE_TO(rpc_context->trace(), "Failed to deserialize 
batch: $0", status.GetDetail());
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/krpc-data-stream-recvr.cc@647
PS5, Line 647:   if (ctx->rpc_context != nullptr)  
TRACE_TO(ctx->rpc_context->trace(), "Batch queue is full");
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/krpc-data-stream-recvr.cc@670
PS5, Line 670: if (ctx->rpc_context != nullptr) 
recvr_->deferred_rpc_tracker()->Release(ctx->rpc_context->GetTransferSize());
line too long (114 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/krpc-data-stream-recvr.cc@675
PS5, Line 675:   if (ctx->rpc_context != nullptr) 
DataStreamService::RespondRpc(status, ctx->response, ctx->rpc_context);
line too long (106 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/krpc-data-stream-sender.cc@695
PS5, Line 695: RETURN_IF_ERROR(parent_->SerializeBatch(batch, 
serialization_batch->get(), !is_local_));
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/local-row-batch-channel.h
File be/src/runtime/local-row-batch-channel.h:

http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/local-row-batch-channel.h@38
PS5, Line 38:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/local-row-batch-channel.cc
File be/src/runtime/local-row-batch-channel.cc:

http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/local-row-batch-channel.cc@50
PS5, Line 50:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/local-row-batch-channel.cc@84
PS5, Line 84:   }
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/local-row-batch-channel.cc@106
PS5, Line 106:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/local-row-batch-channel.cc@173
PS5, Line 173:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/5/be/src/runtime/local-row-batch-channel.cc@182
PS5, Line 182: Status LocalRowBatchChannelManager::RegisterSender(RuntimeState* 
state,
line has trailing whitespace



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 08:18:53 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Csaba Ringhofer (Code Review)
Hello Impala Public Jenkins,

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

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

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

Change subject: WIP: Skip KRPC in local exchanges
..

WIP: Skip KRPC in local exchanges

This is a lot of mess at the moment.
Major TODOs:
- add profile counters
- refactor krpc channel to another class from krpc-data-stream-sender

Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
---
M be/src/exec/exchange-node.cc
M be/src/runtime/CMakeLists.txt
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-mgr.h
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-recvr.h
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.h
A be/src/runtime/local-row-batch-channel.cc
A be/src/runtime/local-row-batch-channel.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M tests/custom_cluster/test_exchange_delays.py
M tests/query_test/test_observability.py
16 files changed, 755 insertions(+), 67 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12584: Add backend config to restrict data file locations for Iceberg tables

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20786 )

Change subject: IMPALA-12584: Add backend config to restrict data file 
locations for Iceberg tables
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I60e3d93b5039dc977417e7b097b3d6ddeda52de4
Gerrit-Change-Number: 20786
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 08:29:01 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12584: Add backend config to restrict data file locations for Iceberg tables

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20786 )

Change subject: IMPALA-12584: Add backend config to restrict data file 
locations for Iceberg tables
..


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I60e3d93b5039dc977417e7b097b3d6ddeda52de4
Gerrit-Change-Number: 20786
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 08:29:02 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12584: Add backend config to restrict data file locations for Iceberg tables

2023-12-18 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20786 )

Change subject: IMPALA-12584: Add backend config to restrict data file 
locations for Iceberg tables
..


Patch Set 5: Code-Review+2

Thanks for fixing this!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I60e3d93b5039dc977417e7b097b3d6ddeda52de4
Gerrit-Change-Number: 20786
Gerrit-PatchSet: 5
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 08:28:40 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 5:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/14767/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 08:38:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 6:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/krpc-data-stream-recvr.cc
File be/src/runtime/krpc-data-stream-recvr.cc:

http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/krpc-data-stream-recvr.cc@343
PS6, Line 343:   if (payload->rpc_context != nullptr) 
TRACE_TO(payload->rpc_context->trace(), "Enqueuing deferred RPC");
line too long (105 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/krpc-data-stream-recvr.cc@493
PS6, Line 493: //TRACE_TO(rpc_context->trace(), "Failed to deserialize 
batch: $0", status.GetDetail());
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/krpc-data-stream-recvr.cc@647
PS6, Line 647:   if (ctx->rpc_context != nullptr)  
TRACE_TO(ctx->rpc_context->trace(), "Batch queue is full");
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/krpc-data-stream-recvr.cc@670
PS6, Line 670: if (ctx->rpc_context != nullptr) 
recvr_->deferred_rpc_tracker()->Release(ctx->rpc_context->GetTransferSize());
line too long (114 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/krpc-data-stream-recvr.cc@675
PS6, Line 675:   if (ctx->rpc_context != nullptr) 
DataStreamService::RespondRpc(status, ctx->response, ctx->rpc_context);
line too long (106 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/krpc-data-stream-sender.cc@695
PS6, Line 695: RETURN_IF_ERROR(parent_->SerializeBatch(batch, 
serialization_batch->get(), !is_local_));
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/local-row-batch-channel.h
File be/src/runtime/local-row-batch-channel.h:

http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/local-row-batch-channel.h@38
PS6, Line 38:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/local-row-batch-channel.cc
File be/src/runtime/local-row-batch-channel.cc:

http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/local-row-batch-channel.cc@50
PS6, Line 50:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/local-row-batch-channel.cc@84
PS6, Line 84:   }
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/local-row-batch-channel.cc@106
PS6, Line 106:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/local-row-batch-channel.cc@173
PS6, Line 173:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/6/be/src/runtime/local-row-batch-channel.cc@182
PS6, Line 182: Status LocalRowBatchChannelManager::RegisterSender(RuntimeState* 
state,
line has trailing whitespace



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 09:28:20 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 09:28:36 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Csaba Ringhofer (Code Review)
Hello Impala Public Jenkins,

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

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

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

Change subject: WIP: Skip KRPC in local exchanges
..

WIP: Skip KRPC in local exchanges

This is a lot of mess at the moment.
Major TODOs:
- add profile counters
- refactor krpc channel to another class from krpc-data-stream-sender

Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
---
M be/src/exec/exchange-node.cc
M be/src/runtime/CMakeLists.txt
M be/src/runtime/data-stream-test.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-mgr.h
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-recvr.h
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.h
A be/src/runtime/local-row-batch-channel.cc
A be/src/runtime/local-row-batch-channel.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M tests/custom_cluster/test_exchange_delays.py
M tests/query_test/test_observability.py
17 files changed, 761 insertions(+), 68 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 09:33:15 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 7:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/krpc-data-stream-recvr.cc
File be/src/runtime/krpc-data-stream-recvr.cc:

http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/krpc-data-stream-recvr.cc@343
PS7, Line 343:   if (payload->rpc_context != nullptr) 
TRACE_TO(payload->rpc_context->trace(), "Enqueuing deferred RPC");
line too long (105 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/krpc-data-stream-recvr.cc@493
PS7, Line 493: //TRACE_TO(rpc_context->trace(), "Failed to deserialize 
batch: $0", status.GetDetail());
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/krpc-data-stream-recvr.cc@647
PS7, Line 647:   if (ctx->rpc_context != nullptr)  
TRACE_TO(ctx->rpc_context->trace(), "Batch queue is full");
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/krpc-data-stream-recvr.cc@670
PS7, Line 670: if (ctx->rpc_context != nullptr) 
recvr_->deferred_rpc_tracker()->Release(ctx->rpc_context->GetTransferSize());
line too long (114 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/krpc-data-stream-recvr.cc@675
PS7, Line 675:   if (ctx->rpc_context != nullptr) 
DataStreamService::RespondRpc(status, ctx->response, ctx->rpc_context);
line too long (106 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/krpc-data-stream-sender.cc@695
PS7, Line 695: RETURN_IF_ERROR(parent_->SerializeBatch(batch, 
serialization_batch->get(), !is_local_));
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/local-row-batch-channel.h
File be/src/runtime/local-row-batch-channel.h:

http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/local-row-batch-channel.h@38
PS7, Line 38:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/local-row-batch-channel.cc
File be/src/runtime/local-row-batch-channel.cc:

http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/local-row-batch-channel.cc@50
PS7, Line 50:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/local-row-batch-channel.cc@84
PS7, Line 84:   }
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/local-row-batch-channel.cc@106
PS7, Line 106:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/local-row-batch-channel.cc@173
PS7, Line 173:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/7/be/src/runtime/local-row-batch-channel.cc@182
PS7, Line 182: Status LocalRowBatchChannelManager::RegisterSender(RuntimeState* 
state,
line has trailing whitespace



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 09:33:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Csaba Ringhofer (Code Review)
Hello Impala Public Jenkins,

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

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

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

Change subject: WIP: Skip KRPC in local exchanges
..

WIP: Skip KRPC in local exchanges

This is a lot of mess at the moment.
Major TODOs:
- add profile counters
- refactor krpc channel to another class from krpc-data-stream-sender

Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
---
M be/src/exec/exchange-node.cc
M be/src/runtime/CMakeLists.txt
M be/src/runtime/data-stream-test.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-mgr.h
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-recvr.h
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.h
A be/src/runtime/local-row-batch-channel.cc
A be/src/runtime/local-row-batch-channel.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M tests/custom_cluster/test_exchange_delays.py
M tests/query_test/test_observability.py
17 files changed, 761 insertions(+), 68 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 6:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 09:55:15 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

2023-12-18 Thread Yifan Zhang (Code Review)
Yifan Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
..


Patch Set 3:

> Patch Set 3:
>
> I think there is a strong reason why Impala trust RowGroups.num_rows more 
> than FileMetaData.num_rows. Maybe there are still invalid parquet files out 
> there that is written with mismatched FileMetaData.num_rows.
>
> See:
> https://issues.apache.org/jira/browse/IMPALA-3943
> https://issues.apache.org/jira/browse/IMPALA-2230

Thanks for your reply, Riza!
I investigated the issues, especially IMPALA-3943. For parquet scans, Impala 
now treats the file with FileMetaData.num_rows=0 as an empty file, see: 
https://github.com/apache/impala/blob/4114fe8db6ec80b2e1679e946555f91ab7043f2e/be/src/exec/parquet/hdfs-parquet-scanner.cc#L895C1-L898.
 And I also found other places that using FileMetaData.num_rows to generate 
query results for metadata only queries, see: 
https://github.com/apache/impala/blob/4114fe8db6ec80b2e1679e946555f91ab7043f2e/be/src/exec/parquet/hdfs-parquet-scanner.cc#L477-L481.
 Besides, Impala-3943 also added warning logs for inconsistency of 
FileMetaData.num_rows and RowGroups.num_rows.

So I think using FileMetaData.num_rows for count star optimizations should be 
acceptable.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Mon, 18 Dec 2023 09:56:11 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 7:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 10:02:44 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12597: Basic Equality delete read support for Iceberg tables

2023-12-18 Thread Gabor Kaszab (Code Review)
Hello Andrew Sherman, Tamas Mate, Daniel Becker, Zoltan Borok-Nagy, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-12597: Basic Equality delete read support for Iceberg 
tables
..

IMPALA-12597: Basic Equality delete read support for Iceberg tables

In general, applying equality deletes is similar to how position
deletes are applied to data files: using a LEFT ANTI JOIN where the
SCAN for the data rows is on the left side while the SCAN for the
delete rows is on the right side of the JOIN. The difference is the
virtual columns and the conjuncts being used.
For equality deletes the data sequence number of a delete file has to
be greater than the data sequence number of the data file being
investigated. This information is added as a virtual column to the
scans and a conjunct is created in the JOIN node to check the relation.
The equality delete fields from the delete files are checked agains the
respective columns of the data SCANS.

This patch makes it possible for Impala to read Iceberg tables with
basic equality delete files. The Iceberg spec gives great flexibility
for engines for writing equality deletes, however in practice Flink,
one of the engines that write EQ-deletes supports only a subset of the
use cases. This patch focuses on reading the EQ-deletes written by
Flink.

The restrictions are the following:
- All equality delete files in a table should have the same equality
  field ID list.
- For partitioned Iceberg tables it is expected that the partition
  values are also written into the equality delete files.
- Tables with equality deletes shouldn't have partition or schema
  evolution.
- Floating point equality columns aren't supported.
- If a malformed equality delete file doesn't have some of the equality
  field IDs then Parquet reader will fill those missing fields with
  NULLs. As a side effect this will drop the rows from the result where
  the corresponding data columns has a null value.
See IMPALA-11388 epic Jira for more details.

Testing:
- Checked if the existing functional_parquet.iceberg_v2_delete_equality
  table can be read successfully.
- Added new test table so that E2E tests can validate correctness.

Change-Id: I2053e6f321c69f1c82059a84a5d99aeaa9814cad
---
M be/src/exec/partitioned-hash-join-builder.h
M be/src/exec/partitioned-hash-join-node.h
M common/thrift/CatalogObjects.thrift
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java
A fe/src/main/java/org/apache/impala/catalog/IcebergDeleteTable.java
A fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/GroupedContentFiles.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/data/README
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/0-0-38a471ff-46f4-4350-85cc-2e7ba946b34c-1.parquet
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/0-0-38a471ff-46f4-4350-85cc-2e7ba946b34c-2.parquet
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/0-0-72709aba-fb15-4bd6-9758-5f39eb9bdcb7-1.parquet
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/0-0-72709aba-fb15-4bd6-9758-5f39eb9bdcb7-2.parquet
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/delete-074a9e19e61b766e-652a169e0001_800513971_data.0.parq
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/0cf1a310-d39c-4c6a-bfef-c3fe33cd0c25-m0.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/0cf1a310-d39c-4c6a-bfef-c3fe33cd0c25-m1.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/3d36bf90-2625-4625-b09b-d4359b979df9-m0.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/3d36bf90-2625-4625-b09b-d4359b979df9-m1.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/bb4b8c07-84e1-421a-bb6c-594f297d118e-m0.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/snap-3802179086205335895-1-3d36bf90-2625-4625-b09b-d4359b979df9.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/snap-8985205515767142888-1-0cf1a310-d39c-4c6a-bfef-c3fe33cd0c25.avro
A

[Impala-ASF-CR] IMPALA-12597: Basic Equality delete read support for Iceberg tables

2023-12-18 Thread Gabor Kaszab (Code Review)
Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20753 )

Change subject: IMPALA-12597: Basic Equality delete read support for Iceberg 
tables
..


Patch Set 9:

PS9 is a rebase with master


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2053e6f321c69f1c82059a84a5d99aeaa9814cad
Gerrit-Change-Number: 20753
Gerrit-PatchSet: 9
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 11:19:18 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight

2023-12-18 Thread Quanlong Huang (Code Review)
Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20799 )

Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
..


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/client-request-state.cc@179
PS5, Line 179: BlockOnWait()
This error message is stale and misleading. This line was added in commit 
bf8e1b8. At that time, BlockOnWait() calls wait_thread.reset(). But now 
wait_thread_ is only reset in Finalized(). I think we should change this to 
"Finalized()".


http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.h@764
PS5, Line 764:   /// in-flight until the query is unregistered.  Until a query 
is in-flight, an attempt
It'd be nice to also mention prestopped_queries.


http://gerrit.cloudera.org:8080/#/c/20799/4/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/20799/4/be/src/service/impala-server.cc@1615
PS4, Line 1615:   
query_handle->session()->prestopped_queries.insert(query_handle->query_id());
> I don't think we'll ever call CloseClientRequestState twice. That would mes
We can check whether the insertion took place and add a warning log if it 
doesn't. The return value of set::insert() can be used.


http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.cc@1616
PS5, Line 1616:   ++query_handle->session()->total_queries;
Could you add a comment here for why we bump the counter when closing a query?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 18 Dec 2023 11:23:51 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12597: Basic Equality delete read support for Iceberg tables

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20753 )

Change subject: IMPALA-12597: Basic Equality delete read support for Iceberg 
tables
..


Patch Set 9:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2053e6f321c69f1c82059a84a5d99aeaa9814cad
Gerrit-Change-Number: 20753
Gerrit-PatchSet: 9
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 11:46:18 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10976: Sync db/table to latest HMS event for all DDL/DMLs

2023-12-18 Thread Quanlong Huang (Code Review)
Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20367 )

Change subject: IMPALA-10976: Sync db/table to latest HMS event for all DDL/DMLs
..


Patch Set 20:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20367/16/fe/src/main/java/org/apache/impala/catalog/Table.java
File fe/src/main/java/org/apache/impala/catalog/Table.java:

http://gerrit.cloudera.org:8080/#/c/20367/16/fe/src/main/java/org/apache/impala/catalog/Table.java@1073
PS16, Line 1073:   lastRefreshEventId_ = eventId;
> Infact, We skip older events based on lastRefreshEventId when 
> event_sync_to_latest_ddl to false even today.

This seems an existing bug. Some events like ALTER_TABLE might skip reloading 
the full metadata and update 'lastRefreshEventId_' to the latest event id. 
Following events will be skipped. Isn't this a bug?


http://gerrit.cloudera.org:8080/#/c/20367/20/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/20367/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3162
PS20, Line 3162: trySyncToLatestEventId(table))) {
If enableSyncToLatestEventOnDdls is true and this is an hdfs table not being 
replicated, we still reload the cache here. Why should we check 
isTableBeingReplicated() here? Isn't the following condition enough?

  if (isIcebergTable || !trySyncToLatestEventId(table))



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia250d0a943838086c187e5cb7c60035e5a564bbf
Gerrit-Change-Number: 20367
Gerrit-PatchSet: 20
Gerrit-Owner: Sai Hemanth Gantasala 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Mon, 18 Dec 2023 11:55:04 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12463: Batch non-consecutive table events in the event processor

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20533 )

Change subject: IMPALA-12463: Batch non-consecutive table events in the event 
processor
..


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I849c0306bc46080ee4059854f42d9c217a89b905
Gerrit-Change-Number: 20533
Gerrit-PatchSet: 8
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 12:33:20 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12584: Add backend config to restrict data file locations for Iceberg tables

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20786 )

Change subject: IMPALA-12584: Add backend config to restrict data file 
locations for Iceberg tables
..


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I60e3d93b5039dc977417e7b097b3d6ddeda52de4
Gerrit-Change-Number: 20786
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 13:00:40 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12584: Add backend config to restrict data file locations for Iceberg tables

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20786 )

Change subject: IMPALA-12584: Add backend config to restrict data file 
locations for Iceberg tables
..

IMPALA-12584: Add backend config to restrict data file locations for Iceberg 
tables

This change adds backend flag 'iceberg_restrict_data_file_location',
when the flag is set to 'true', Impala will raise an error when at least
one data file of an Iceberg table is outside of the table directory.
The default value of the flag is 'false'.

Tests:
 - custom-cluster test added to validate both states of the flag

Change-Id: I60e3d93b5039dc977417e7b097b3d6ddeda52de4
Reviewed-on: http://gerrit.cloudera.org:8080/20786
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
A tests/custom_cluster/test_iceberg_strict_data.py
6 files changed, 68 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I60e3d93b5039dc977417e7b097b3d6ddeda52de4
Gerrit-Change-Number: 20786
Gerrit-PatchSet: 7
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12606: Sporadic failures around query test.test queries.TestQueries.test intersect

2023-12-18 Thread Zoltan Borok-Nagy (Code Review)
Hello Csaba Ringhofer, Michael Smith, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12606: Sporadic failures around 
query_test.test_queries.TestQueries.test_intersect
..

IMPALA-12606: Sporadic failures around 
query_test.test_queries.TestQueries.test_intersect

test_intersect failed when ASYNC_CODEGEN was enabled. This happened
because we were using codegened 'ProcessProbeBatch' in the HASH JOIN
operator with non-codegened InsertBatch/ProcessBuildBatch at the Builder
side, or vice versa.

Only the NULL StringValues were hit by the bug, turned out NULLs are
handled differently in the hash table. We are using the
HashUtil::FNV_SEED number to represent NULL values. This number was
chosen arbitrarily, we just wanted to use some random value.

In the codegened path we invoke StringValue::Assign(ptr, len) with both
params being HashUtil::FNV_SEED. HashUtil::FNV_SEED is a negative value
in int32_t, so StringValue::Assign(ptr, len) stored 0 as len actually,
and not HashUtil::FNV_SEED. This is needed to be resilient against
invalid values in corrupt files.

In non-codegened path we are creating a StringValue object by
reinterpret casting a [HashUtil::FNV_SEED, HashUtil::FNV_SEED, ...]
array to StringValue. Then in RawValue::WriteNonNullPrimitive() we
invoke StringValue::Assign(StringValue&) that just memcopies the
parameter to this. It cannot check for negative values, because the
parameter StringValue might be a valid small string.

To sum up, this is how a NULL string was represented in the HashTable:
* Codegen path: ptr = HashUtil::FNV_SEED, len = 0
* Non-codegen path: ptr = HashUtil::FNV_SEED, len = HashUtil::FNV_SEED

This is why the hash join operator was working incorrectly on NULL
String values when some parts of it used Codegen'ed path while other
parts were using the non-codegened path.

To fix the issue, I introduced UnsafeAssign(ptr, len) which doesn't
do any checks for 'ptr' or 'len', so we have the same StringValue for
objects for NULLs at both the codegened and non-codegened paths.

Testing:
 * Executed TestQueries.test_intersect multiple times

Change-Id: I6b855c59808db80fd7ac596ce338fc4c3c9c7667
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/hash-table.cc
M be/src/runtime/smallable-string.h
M be/src/runtime/string-value-ir.cc
M be/src/runtime/string-value.h
5 files changed, 21 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6b855c59808db80fd7ac596ce338fc4c3c9c7667
Gerrit-Change-Number: 20793
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12606: Sporadic failures around query test.test queries.TestQueries.test intersect

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20793 )

Change subject: IMPALA-12606: Sporadic failures around 
query_test.test_queries.TestQueries.test_intersect
..


Patch Set 6:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b855c59808db80fd7ac596ce338fc4c3c9c7667
Gerrit-Change-Number: 20793
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:08:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 8:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:15:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 8:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/krpc-data-stream-recvr.cc
File be/src/runtime/krpc-data-stream-recvr.cc:

http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/krpc-data-stream-recvr.cc@343
PS8, Line 343:   if (payload->rpc_context != nullptr) 
TRACE_TO(payload->rpc_context->trace(), "Enqueuing deferred RPC");
line too long (105 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/krpc-data-stream-recvr.cc@493
PS8, Line 493: //TRACE_TO(rpc_context->trace(), "Failed to deserialize 
batch: $0", status.GetDetail());
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/krpc-data-stream-recvr.cc@648
PS8, Line 648:   if (ctx->rpc_context != nullptr)  
TRACE_TO(ctx->rpc_context->trace(), "Batch queue is full");
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/krpc-data-stream-recvr.cc@671
PS8, Line 671: if (ctx->rpc_context != nullptr) 
recvr_->deferred_rpc_tracker()->Release(ctx->rpc_context->GetTransferSize());
line too long (114 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/krpc-data-stream-recvr.cc@676
PS8, Line 676:   if (ctx->rpc_context != nullptr) 
DataStreamService::RespondRpc(status, ctx->response, ctx->rpc_context);
line too long (106 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/krpc-data-stream-sender.cc@695
PS8, Line 695: RETURN_IF_ERROR(parent_->SerializeBatch(batch, 
serialization_batch->get(), !is_local_));
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/local-row-batch-channel.h
File be/src/runtime/local-row-batch-channel.h:

http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/local-row-batch-channel.h@38
PS8, Line 38:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/local-row-batch-channel.cc
File be/src/runtime/local-row-batch-channel.cc:

http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/local-row-batch-channel.cc@50
PS8, Line 50:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/local-row-batch-channel.cc@93
PS8, Line 93:   }
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/local-row-batch-channel.cc@115
PS8, Line 115:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/local-row-batch-channel.cc@182
PS8, Line 182:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/8/be/src/runtime/local-row-batch-channel.cc@191
PS8, Line 191: Status LocalRowBatchChannelManager::RegisterSender(RuntimeState* 
state,
line has trailing whitespace



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:16:21 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

2023-12-18 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
9 files changed, 256 insertions(+), 56 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Csaba Ringhofer (Code Review)
Hello Impala Public Jenkins,

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

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

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

Change subject: WIP: Skip KRPC in local exchanges
..

WIP: Skip KRPC in local exchanges

This is a lot of mess at the moment.
Major TODOs:
- add profile counters
- refactor krpc channel to another class from krpc-data-stream-sender

Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
---
M be/src/exec/exchange-node.cc
M be/src/runtime/CMakeLists.txt
M be/src/runtime/data-stream-test.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-mgr.h
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-recvr.h
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.h
A be/src/runtime/local-row-batch-channel.cc
A be/src/runtime/local-row-batch-channel.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M tests/custom_cluster/test_exchange_delays.py
M tests/query_test/test_observability.py
17 files changed, 772 insertions(+), 68 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/20810/8
--
To view, visit http://gerrit.cloudera.org:8080/20810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 7: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10061/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:20:38 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12313: (part 3) Add UPDATE support for Iceberg tables.

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20760 )

Change subject: IMPALA-12313: (part 3) Add UPDATE support for Iceberg tables.
..


Patch Set 13:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2bb97b4454165a292975d88dc9c23adb22ff7315
Gerrit-Change-Number: 20760
Gerrit-PatchSet: 13
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:36:03 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12313: (part 3) Add UPDATE support for Iceberg tables.

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20760 )

Change subject: IMPALA-12313: (part 3) Add UPDATE support for Iceberg tables.
..


Patch Set 13: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2bb97b4454165a292975d88dc9c23adb22ff7315
Gerrit-Change-Number: 20760
Gerrit-PatchSet: 13
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:36:02 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 9:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/krpc-data-stream-recvr.cc
File be/src/runtime/krpc-data-stream-recvr.cc:

http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/krpc-data-stream-recvr.cc@343
PS9, Line 343:   if (payload->rpc_context != nullptr) 
TRACE_TO(payload->rpc_context->trace(), "Enqueuing deferred RPC");
line too long (105 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/krpc-data-stream-recvr.cc@493
PS9, Line 493: //TRACE_TO(rpc_context->trace(), "Failed to deserialize 
batch: $0", status.GetDetail());
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/krpc-data-stream-recvr.cc@648
PS9, Line 648:   if (ctx->rpc_context != nullptr)  
TRACE_TO(ctx->rpc_context->trace(), "Batch queue is full");
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/krpc-data-stream-recvr.cc@671
PS9, Line 671: if (ctx->rpc_context != nullptr) 
recvr_->deferred_rpc_tracker()->Release(ctx->rpc_context->GetTransferSize());
line too long (114 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/krpc-data-stream-recvr.cc@676
PS9, Line 676:   if (ctx->rpc_context != nullptr) 
DataStreamService::RespondRpc(status, ctx->response, ctx->rpc_context);
line too long (106 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/krpc-data-stream-sender.cc@695
PS9, Line 695: RETURN_IF_ERROR(parent_->SerializeBatch(batch, 
serialization_batch->get(), !is_local_));
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/local-row-batch-channel.h
File be/src/runtime/local-row-batch-channel.h:

http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/local-row-batch-channel.h@38
PS9, Line 38:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/local-row-batch-channel.cc
File be/src/runtime/local-row-batch-channel.cc:

http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/local-row-batch-channel.cc@50
PS9, Line 50:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/local-row-batch-channel.cc@93
PS9, Line 93:   }
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/local-row-batch-channel.cc@115
PS9, Line 115:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/local-row-batch-channel.cc@182
PS9, Line 182:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20810/9/be/src/runtime/local-row-batch-channel.cc@191
PS9, Line 191: Status LocalRowBatchChannelManager::RegisterSender(RuntimeState* 
state,
line has trailing whitespace



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:41:08 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 8:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:41:15 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 6:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:42:15 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Csaba Ringhofer (Code Review)
Hello Impala Public Jenkins,

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

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

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

Change subject: WIP: Skip KRPC in local exchanges
..

WIP: Skip KRPC in local exchanges

This is a lot of mess at the moment.
Major TODOs:
- add profile counters
- refactor krpc channel to another class from krpc-data-stream-sender

Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
---
M be/src/exec/exchange-node.cc
M be/src/runtime/CMakeLists.txt
M be/src/runtime/data-stream-test.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-mgr.h
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-recvr.h
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.h
A be/src/runtime/local-row-batch-channel.cc
A be/src/runtime/local-row-batch-channel.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M tests/custom_cluster/test_exchange_delays.py
M tests/query_test/test_observability.py
17 files changed, 772 insertions(+), 68 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/20810/9
--
To view, visit http://gerrit.cloudera.org:8080/20810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12500: Reduce flakyness of test global exchange counters

2023-12-18 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20811


Change subject: IMPALA-12500: Reduce flakyness of test_global_exchange_counters
..

IMPALA-12500: Reduce flakyness of test_global_exchange_counters

Ran the tests repeatedly and the failures I saw were only 0.01 off.
Ignoring the last digit of ExchangeScanRatio seems to be enough
to make the test consistently green.

Change-Id: I530b690128d4c55a793440425dd39c6523741bf7
---
M tests/query_test/test_observability.py
1 file changed, 2 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I530b690128d4c55a793440425dd39c6523741bf7
Gerrit-Change-Number: 20811
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 


[Impala-ASF-CR] IMPALA-12597: Basic Equality delete read support for Iceberg tables

2023-12-18 Thread Gabor Kaszab (Code Review)
Hello Andrew Sherman, Tamas Mate, Daniel Becker, Zoltan Borok-Nagy, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-12597: Basic Equality delete read support for Iceberg 
tables
..

IMPALA-12597: Basic Equality delete read support for Iceberg tables

In general, applying equality deletes is similar to how position
deletes are applied to data files: using a LEFT ANTI JOIN where the
SCAN for the data rows is on the left side while the SCAN for the
delete rows is on the right side of the JOIN. The difference is the
virtual columns and the conjuncts being used.
For equality deletes the data sequence number of a delete file has to
be greater than the data sequence number of the data file being
investigated. This information is added as a virtual column to the
scans and a conjunct is created in the JOIN node to check the relation.
The equality delete fields from the delete files are checked agains the
respective columns of the data SCANS.

This patch makes it possible for Impala to read Iceberg tables with
basic equality delete files. The Iceberg spec gives great flexibility
for engines for writing equality deletes, however in practice Flink,
one of the engines that write EQ-deletes supports only a subset of the
use cases. This patch focuses on reading the EQ-deletes written by
Flink.

The restrictions are the following:
- All equality delete files in a table should have the same equality
  field ID list.
- For partitioned Iceberg tables it is expected that the partition
  values are also written into the equality delete files.
- Tables with equality deletes shouldn't have partition or schema
  evolution.
- Floating point equality columns aren't supported.
- If a malformed equality delete file doesn't have some of the equality
  field IDs then Parquet reader will fill those missing fields with
  NULLs. As a side effect this will drop the rows from the result where
  the corresponding data columns have a null value.
See IMPALA-11388 epic Jira for more details.

Testing:
- Checked if the existing functional_parquet.iceberg_v2_delete_equality
  table can be read successfully.
- Added new test tables so that E2E tests can validate correctness.

Change-Id: I2053e6f321c69f1c82059a84a5d99aeaa9814cad
---
M be/src/exec/partitioned-hash-join-builder.h
M be/src/exec/partitioned-hash-join-node.h
M common/thrift/CatalogObjects.thrift
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java
A fe/src/main/java/org/apache/impala/catalog/IcebergDeleteTable.java
A fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/GroupedContentFiles.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/data/README
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/0-0-38a471ff-46f4-4350-85cc-2e7ba946b34c-1.parquet
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/0-0-38a471ff-46f4-4350-85cc-2e7ba946b34c-2.parquet
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/0-0-72709aba-fb15-4bd6-9758-5f39eb9bdcb7-1.parquet
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/0-0-72709aba-fb15-4bd6-9758-5f39eb9bdcb7-2.parquet
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/delete-074a9e19e61b766e-652a169e0001_800513971_data.0.parq
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/0cf1a310-d39c-4c6a-bfef-c3fe33cd0c25-m0.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/0cf1a310-d39c-4c6a-bfef-c3fe33cd0c25-m1.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/3d36bf90-2625-4625-b09b-d4359b979df9-m0.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/3d36bf90-2625-4625-b09b-d4359b979df9-m1.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/bb4b8c07-84e1-421a-bb6c-594f297d118e-m0.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/snap-3802179086205335895-1-3d36bf90-2625-4625-b09b-d4359b979df9.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/snap-8985205515767142888-1-0cf1a310-d39c-4c6a-bfef-c3fe33cd0c25.avr

[Impala-ASF-CR] IMPALA-12500: Reduce flakyness of test global exchange counters

2023-12-18 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20811 )

Change subject: IMPALA-12500: Reduce flakyness of test_global_exchange_counters
..


Patch Set 1: Code-Review+2

LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I530b690128d4c55a793440425dd39c6523741bf7
Gerrit-Change-Number: 20811
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:51:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12597: Basic Equality delete read support for Iceberg tables

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20753 )

Change subject: IMPALA-12597: Basic Equality delete read support for Iceberg 
tables
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20753/10/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/20753/10/tests/query_test/test_iceberg.py@1285
PS10, Line 1285: +
flake8: W504 line break after binary operator



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2053e6f321c69f1c82059a84a5d99aeaa9814cad
Gerrit-Change-Number: 20753
Gerrit-PatchSet: 10
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:51:59 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12606: Sporadic failures around query test.test queries.TestQueries.test intersect

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20793 )

Change subject: IMPALA-12606: Sporadic failures around 
query_test.test_queries.TestQueries.test_intersect
..


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b855c59808db80fd7ac596ce338fc4c3c9c7667
Gerrit-Change-Number: 20793
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:53:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12500: Reduce flakyness of test global exchange counters

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20811 )

Change subject: IMPALA-12500: Reduce flakyness of test_global_exchange_counters
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I530b690128d4c55a793440425dd39c6523741bf7
Gerrit-Change-Number: 20811
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:55:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12500: Reduce flakyness of test global exchange counters

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20811 )

Change subject: IMPALA-12500: Reduce flakyness of test_global_exchange_counters
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I530b690128d4c55a793440425dd39c6523741bf7
Gerrit-Change-Number: 20811
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 14:55:36 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 9:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:00:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

2023-12-18 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 6: Code-Review+1

(2 comments)

Nice work, Tamas!

http://gerrit.cloudera.org:8080/#/c/20759/6/fe/src/main/java/org/apache/impala/analysis/FromClause.java
File fe/src/main/java/org/apache/impala/analysis/FromClause.java:

http://gerrit.cloudera.org:8080/#/c/20759/6/fe/src/main/java/org/apache/impala/analysis/FromClause.java@96
PS6, Line 96: if (tblRef.getResolvedPath().getRootTable() instanceof 
IcebergMetadataTable) {
:   throw new AnalysisException("Querying collection types 
(ARRAY/MAP) is not " +
:   "supported for Iceberg Metadata tables. 
(IMPALA-12610, IMPALA-12611)");
: }
nit: for readability, can we put this check into its own method?


http://gerrit.cloudera.org:8080/#/c/20759/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
File 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test:

http://gerrit.cloudera.org:8080/#/c/20759/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@540
PS6, Line 540: 
Thanks for adding these tests! Could you please add one with this JOIN syntax:

 select item from functional_parquet.iceberg_query_metadata.all_files a, 
a.equality_ids e, e.delete_ids;



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:05:04 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12597: Basic Equality delete read support for Iceberg tables

2023-12-18 Thread Gabor Kaszab (Code Review)
Hello Andrew Sherman, Tamas Mate, Daniel Becker, Zoltan Borok-Nagy, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-12597: Basic Equality delete read support for Iceberg 
tables
..

IMPALA-12597: Basic Equality delete read support for Iceberg tables

In general, applying equality deletes is similar to how position
deletes are applied to data files: using a LEFT ANTI JOIN where the
SCAN for the data rows is on the left side while the SCAN for the
delete rows is on the right side of the JOIN. The difference is the
virtual columns and the conjuncts being used.
For equality deletes the data sequence number of a delete file has to
be greater than the data sequence number of the data file being
investigated. This information is added as a virtual column to the
scans and a conjunct is created in the JOIN node to check the relation.
The equality delete fields from the delete files are checked agains the
respective columns of the data SCANS.

This patch makes it possible for Impala to read Iceberg tables with
basic equality delete files. The Iceberg spec gives great flexibility
for engines for writing equality deletes, however in practice Flink,
one of the engines that write EQ-deletes supports only a subset of the
use cases. This patch focuses on reading the EQ-deletes written by
Flink.

The restrictions are the following:
- All equality delete files in a table should have the same equality
  field ID list.
- For partitioned Iceberg tables it is expected that the partition
  values are also written into the equality delete files.
- Tables with equality deletes shouldn't have partition or schema
  evolution.
- Floating point equality columns aren't supported.
- If a malformed equality delete file doesn't have some of the equality
  field IDs then Parquet reader will fill those missing fields with
  NULLs. As a side effect this will drop the rows from the result where
  the corresponding data columns have a null value.
See IMPALA-11388 epic Jira for more details.

Testing:
- Checked if the existing functional_parquet.iceberg_v2_delete_equality
  table can be read successfully.
- Added new test tables so that E2E tests can validate correctness.

Change-Id: I2053e6f321c69f1c82059a84a5d99aeaa9814cad
---
M be/src/exec/partitioned-hash-join-builder.h
M be/src/exec/partitioned-hash-join-node.h
M common/thrift/CatalogObjects.thrift
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java
A fe/src/main/java/org/apache/impala/catalog/IcebergDeleteTable.java
A fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/GroupedContentFiles.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/data/README
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/0-0-38a471ff-46f4-4350-85cc-2e7ba946b34c-1.parquet
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/0-0-38a471ff-46f4-4350-85cc-2e7ba946b34c-2.parquet
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/0-0-72709aba-fb15-4bd6-9758-5f39eb9bdcb7-1.parquet
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/0-0-72709aba-fb15-4bd6-9758-5f39eb9bdcb7-2.parquet
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/data/delete-074a9e19e61b766e-652a169e0001_800513971_data.0.parq
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/0cf1a310-d39c-4c6a-bfef-c3fe33cd0c25-m0.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/0cf1a310-d39c-4c6a-bfef-c3fe33cd0c25-m1.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/3d36bf90-2625-4625-b09b-d4359b979df9-m0.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/3d36bf90-2625-4625-b09b-d4359b979df9-m1.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/bb4b8c07-84e1-421a-bb6c-594f297d118e-m0.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/snap-3802179086205335895-1-3d36bf90-2625-4625-b09b-d4359b979df9.avro
A 
testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_v2_delete_both_eq_and_pos/metadata/snap-8985205515767142888-1-0cf1a310-d39c-4c6a-bfef-c3fe33cd0c25.avr

[Impala-ASF-CR] IMPALA-12597: Basic Equality delete read support for Iceberg tables

2023-12-18 Thread Gabor Kaszab (Code Review)
Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20753 )

Change subject: IMPALA-12597: Basic Equality delete read support for Iceberg 
tables
..


Patch Set 10:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/20753/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20753/8//COMMIT_MSG@14
PS8, Line 14: data sequence number of a delete file has to
: be greater than the data sequence number of the data file being
: investigated
> nit: improvement idea for the future:
This optimization sounds reasonable for me. created IMPALA-12649 for this. 
(copy-pasted your explanation to the ticket:) )


http://gerrit.cloudera.org:8080/#/c/20753/8//COMMIT_MSG@39
PS8, Line 39: hav
> have
Done


http://gerrit.cloudera.org:8080/#/c/20753/8//COMMIT_MSG@45
PS8, Line 45: table
> nit: tables
Done


http://gerrit.cloudera.org:8080/#/c/20753/8/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:

http://gerrit.cloudera.org:8080/#/c/20753/8/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@504
PS8, Line 504:   equalityDeleteFiles_.add(delFileDesc.first);
 :   addEqualityIds(delFile.equalityFieldIds());
 : } else {
 :   Preconditions.checkState(delFile.content() == 
FileContent.POSITION_DELETES);
 :   positionDeleteFiles_.add(delFileDesc.first);
 : }
 :   }
 : }
 :   }
> nit: for readability, this could go to its own method.
Done


http://gerrit.cloudera.org:8080/#/c/20753/8/testdata/data/README
File testdata/data/README:

http://gerrit.cloudera.org:8080/#/c/20753/8/testdata/data/README@776
PS8, Line 776: when running a DELETE FROM statement
> Could we also hack INSERT INTO? In that case Impala would just write the Pa
I think that is also a valid way to hack these eq-deletes. Next time I'll try 
it this way :)


http://gerrit.cloudera.org:8080/#/c/20753/8/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/20753/8/tests/query_test/test_iceberg.py@1262
PS8, Line 1262: e
> nit: we usually don't add space here
Done


http://gerrit.cloudera.org:8080/#/c/20753/8/tests/query_test/test_iceberg.py@1264
PS8, Line 1264:
  :   @SkipIfDockerizedCluster.internal_hostname
  :   @SkipIf.hardcoded_uris
  :   def test_multiple_equality_ids(self, unique_database):
  : """This test loads an Iceberg table that has 2 equality 
delete files with different
  : equality ID lists. A query on such a table fails due to 
lack of support."""
  : SRC_DIR = os.path.join(os.environ['IMPALA_HOME'],
  : "testdata/data/iceberg_test/hadoop_catalog/ice/"
  : "iceberg_v2_delete_different_equality_ids")
  : DST_DIR = 
"/test-warehouse/iceberg_test/hadoop_catalog/ice/" \
  : "iceberg_v2_delete_different_equality_ids"
> Can we use 'file_utils.create_iceberg_table_from_directory'? With equality
Good Idea. I gave this a try, however I realized that for this particular table 
it's not possible to actually load it as there are different equality fields ID 
lists, so even an ALTER TABLE after the CREATE TABLE would give an error. So in 
practice for hadoop catalog I had to add a number of 'is hadoop catalog' kind 
of checks inot the function plus I had to also make an option to not run the 
last ALTER TABLE statement in the function. I found this too much complexity 
being added for this particular table. In general for adding tables with this 
function into hadoop catalog would be great but it wouldn;t work in this case.


http://gerrit.cloudera.org:8080/#/c/20753/8/tests/query_test/test_iceberg.py@1278
PS8, Line 1278:
> Is there a reason we create an external table?
If I create it as a non-external table I get an error saying that the table 
already exists. Anyway, create_iceberg_table_from_directory() also creates an 
external table


http://gerrit.cloudera.org:8080/#/c/20753/8/tests/query_test/test_iceberg.py@1290
PS8, Line 1290:
  :   err = self.execute_query_e
> What are these lists?
These are the mismatching equality ID lists. Added a comment for more clarity.


http://gerrit.cloudera.org:8080/#/c/20753/10/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/20753/10/tests/query_test/test_iceberg.py@1267
PS10, Line 1267:   def test_multiple_equality_ids(self, unique_database):
Added the test table to unique_database so that I can save on level of 
try-catch block



--
To view, visit http://gerrit.cloudera.org:8080/20753
To unsubscribe, visit http://ger

[Impala-ASF-CR] IMPALA-12593: Split test drop partition test table

2023-12-18 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20746 )

Change subject: IMPALA-12593: Split test_drop_partition test table
..


Patch Set 1: Code-Review+2

looks good to me!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc080c6bde091e17d20adc95da998e36c484f768
Gerrit-Change-Number: 20746
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:14:52 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12500: Reduce flakyness of test global exchange counters

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20811 )

Change subject: IMPALA-12500: Reduce flakyness of test_global_exchange_counters
..


Patch Set 1:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I530b690128d4c55a793440425dd39c6523741bf7
Gerrit-Change-Number: 20811
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:17:50 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12593: Split test drop partition test table

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20746 )

Change subject: IMPALA-12593: Split test_drop_partition test table
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc080c6bde091e17d20adc95da998e36c484f768
Gerrit-Change-Number: 20746
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:24:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12597: Basic Equality delete read support for Iceberg tables

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20753 )

Change subject: IMPALA-12597: Basic Equality delete read support for Iceberg 
tables
..


Patch Set 10:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2053e6f321c69f1c82059a84a5d99aeaa9814cad
Gerrit-Change-Number: 20753
Gerrit-PatchSet: 10
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:24:37 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12593: Split test drop partition test table

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20746 )

Change subject: IMPALA-12593: Split test_drop_partition test table
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc080c6bde091e17d20adc95da998e36c484f768
Gerrit-Change-Number: 20746
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:24:07 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

2023-12-18 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
9 files changed, 270 insertions(+), 56 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

2023-12-18 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 7:

(2 comments)

Thanks Zoltan!

http://gerrit.cloudera.org:8080/#/c/20759/6/fe/src/main/java/org/apache/impala/analysis/FromClause.java
File fe/src/main/java/org/apache/impala/analysis/FromClause.java:

http://gerrit.cloudera.org:8080/#/c/20759/6/fe/src/main/java/org/apache/impala/analysis/FromClause.java@96
PS6, Line 96: checkIcebergCollectionSupport(tblRef);
: checkTopLevelComplexAcidScan(analyzer, 
(CollectionTableRef)tblRef);
: if (firstZippingUnnestRef != null && 
tblRef.isZippingUnnest() &&
:
> nit: for readability, can we put this check into its own method?
Done


http://gerrit.cloudera.org:8080/#/c/20759/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
File 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test:

http://gerrit.cloudera.org:8080/#/c/20759/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@540
PS6, Line 540: 
> Thanks for adding these tests! Could you please add one with this JOIN synt
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:27:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12597: Basic Equality delete read support for Iceberg tables

2023-12-18 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20753 )

Change subject: IMPALA-12597: Basic Equality delete read support for Iceberg 
tables
..


Patch Set 8:

(3 comments)

I only had one comment that might require some additional change: the one about 
'rewrite-iceberg-metadata.py'

Otherwise I'm ready to +2 it.

http://gerrit.cloudera.org:8080/#/c/20753/8/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/20753/8/tests/query_test/test_iceberg.py@1264
PS8, Line 1264: SRC_DIR = os.path.join(os.environ['IMPALA_HOME'],
  : "testdata/data/iceberg_test/hadoop_catalog/ice/"
  : "iceberg_v2_delete_different_equality_ids")
  : DST_DIR = 
"/test-warehouse/iceberg_test/hadoop_catalog/ice/" \
  : "iceberg_v2_delete_different_equality_ids"
  :
  : try:
  :   self.filesystem_client.make_dir(DST_DIR, permission=777)
  :
  :   
self.filesystem_client.copy_from_local(os.path.join(SRC_DIR, "data"), DST_DIR)
  :   
self.filesystem_client.copy_from_local(os.path.join(SRC_DIR, "metadata"), 
DST_DIR)
> Good Idea. I gave this a try, however I realized that for this particular t
I see, thanks for giving it a try.

Maybe invoking testdata/bin/rewrite-iceberg-metadata.py could be added, so this 
test could run in non-HDFS environments as well.


http://gerrit.cloudera.org:8080/#/c/20753/8/tests/query_test/test_iceberg.py@1278
PS8, Line 1278: external
> If I create it as a non-external table I get an error saying that the table
I see.
I also find it strange that create_iceberg_table_from_directory() creates 
external tables, especially that right after table creation it sets table 
property 'external.table.purge'='True'


http://gerrit.cloudera.org:8080/#/c/20753/8/tests/query_test/test_iceberg.py@1290
PS8, Line 1290: assert "[1, 2]" in str(err)
  : assert "[1]" in str(err)
> These are the mismatching equality ID lists. Added a comment for more clari
Thanks!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2053e6f321c69f1c82059a84a5d99aeaa9814cad
Gerrit-Change-Number: 20753
Gerrit-PatchSet: 8
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:30:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

2023-12-18 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 7:

I opened IMPALA-12651 for binary type as string support.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:39:13 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12597: Basic Equality delete read support for Iceberg tables

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20753 )

Change subject: IMPALA-12597: Basic Equality delete read support for Iceberg 
tables
..


Patch Set 11:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2053e6f321c69f1c82059a84a5d99aeaa9814cad
Gerrit-Change-Number: 20753
Gerrit-PatchSet: 11
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:45:19 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12628: iceberg negative test fail in non-HDFS environments

2023-12-18 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20812


Change subject: IMPALA-12628: iceberg negative test fail in non-HDFS 
environments
..

IMPALA-12628: iceberg negative test fail in non-HDFS environments

There's a HIVE_QUERY section in iceberg-negative.test. Such sections
use Hive to execute statements. In non-HDFS environments like S3,
Ozone, etc. Hive is not configured properly, so we cannot use
it.

This patch adds the ' IS_HDFS_ONLY' section to the relevant
test blocks (The HIVE_QUERY that creates the table, and the
Impala test that checks an error message).

Change-Id: I7987724f11d0c88d09577f52294ecc10d8ce39ce
---
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
1 file changed, 2 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7987724f11d0c88d09577f52294ecc10d8ce39ce
Gerrit-Change-Number: 20812
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

2023-12-18 Thread Gabor Kaszab (Code Review)
Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 7: Code-Review+2

(1 comment)

he change looks great, Tamas! One minor nit, but other than that +2

http://gerrit.cloudera.org:8080/#/c/20759/7/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
File be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc:

http://gerrit.cloudera.org:8080/#/c/20759/7/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@105
PS7, Line 105: childs
nit: children



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:56:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

2023-12-18 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 8: Code-Review+2

(1 comment)

Thanks Gabor!
Carrying over the +2.

http://gerrit.cloudera.org:8080/#/c/20759/7/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
File be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc:

http://gerrit.cloudera.org:8080/#/c/20759/7/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@105
PS7, Line 105: childr
> nit: children
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:58:56 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12628: iceberg negative test fail in non-HDFS environments

2023-12-18 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20812 )

Change subject: IMPALA-12628: iceberg negative test fail in non-HDFS 
environments
..

IMPALA-12628: iceberg negative test fail in non-HDFS environments

There's a HIVE_QUERY section in iceberg-negative.test. Such sections
use Hive to execute statements. In non-HDFS environments like S3,
Ozone, etc. Hive is not configured properly, so we cannot use
it.

This patch adds the ' IS_HDFS_ONLY' section to the relevant
test blocks (The HIVE_QUERY that creates the table, and the
Impala test that checks an error message).

Change-Id: I7987724f11d0c88d09577f52294ecc10d8ce39ce
Reviewed-on: http://gerrit.cloudera.org:8080/20812
Reviewed-by: Csaba Ringhofer 
Tested-by: Zoltan Borok-Nagy 
---
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
1 file changed, 2 insertions(+), 0 deletions(-)

Approvals:
  Csaba Ringhofer: Looks good to me, approved
  Zoltan Borok-Nagy: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7987724f11d0c88d09577f52294ecc10d8ce39ce
Gerrit-Change-Number: 20812
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12628: iceberg negative test fail in non-HDFS environments

2023-12-18 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20812 )

Change subject: IMPALA-12628: iceberg negative test fail in non-HDFS 
environments
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7987724f11d0c88d09577f52294ecc10d8ce39ce
Gerrit-Change-Number: 20812
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:53:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 7:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:59:01 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12628: iceberg negative test fail in non-HDFS environments

2023-12-18 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20812 )

Change subject: IMPALA-12628: iceberg negative test fail in non-HDFS 
environments
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7987724f11d0c88d09577f52294ecc10d8ce39ce
Gerrit-Change-Number: 20812
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:55:04 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

2023-12-18 Thread Tamas Mate (Code Review)
Hello Gabor Kaszab, Zoltan Borok-Nagy, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
9 files changed, 270 insertions(+), 56 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20759/8
--
To view, visit http://gerrit.cloudera.org:8080/20759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 9: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 16:05:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 16:05:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12650: Skip some queries in test create unicode table on non-HFDS envs

2023-12-18 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20813


Change subject: IMPALA-12650: Skip some queries in test_create_unicode_table on 
non-HFDS envs
..

IMPALA-12650: Skip some queries in test_create_unicode_table on non-HFDS envs

Impala's non-HDFS test environment has no Hive running, making
these tests fail. Added IS_HDFS_ONLY section for queries that
need Hive.

Change-Id: Ied4e5e295cf577cc45adc8d3b73d8682553d1b14
---
M testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test
1 file changed, 4 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ied4e5e295cf577cc45adc8d3b73d8682553d1b14
Gerrit-Change-Number: 20813
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 


[Impala-ASF-CR] IMPALA-12628: iceberg negative test fail in non-HDFS environments

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20812 )

Change subject: IMPALA-12628: iceberg negative test fail in non-HDFS 
environments
..


Patch Set 1:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7987724f11d0c88d09577f52294ecc10d8ce39ce
Gerrit-Change-Number: 20812
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 16:30:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12606: Sporadic failures around query test.test queries.TestQueries.test intersect

2023-12-18 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20793 )

Change subject: IMPALA-12606: Sporadic failures around 
query_test.test_queries.TestQueries.test_intersect
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b855c59808db80fd7ac596ce338fc4c3c9c7667
Gerrit-Change-Number: 20793
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 16:31:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 8:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 16:52:36 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-12-18 Thread Riza Suminto (Code Review)
Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20612 )

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..


Patch Set 16:

ps16 is a rebase.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
Gerrit-Change-Number: 20612
Gerrit-PatchSet: 16
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 18 Dec 2023 16:54:32 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-12-18 Thread Riza Suminto (Code Review)
Hello Kurt Deschler, Abhishek Rawat, Csaba Ringhofer, Michael Smith, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..

IMPALA-3825: Delegate runtime filter aggregation to some executors

IMPALA-4400 improve the runtime filter by aggregating runtime filters
locally before sending filter update to the coordinator and sharing a
single RuntimeFilterBank for all fragment instances in a query. However,
local filter aggregation is still insufficient if the number of nodes in
an impala cluster is large. For example, in a cluster of around 700
impalad backends, aggregation of 1 MB bloom filter updates in the
coordinator can exceed more than 1 second.

This patch aims to reduce coordinator load and speed up runtime filter
aggregation by doing intermediate aggregation in a few designated impala
backends before doing final aggregation and publishing in the
coordinator. Query option MAX_NUM_FILTERS_AGGREGATED_PER_HOST is added
to control this feature. Given N as the number of backend executors
excluding the coordinator, the selected number of intermediate
aggregators M = ceil(N / MAX_NUM_FILTERS_AGGREGATED_PER_HOST). Setting
MAX_NUM_FILTERS_AGGREGATED_PER_HOST <= 1 will disable the intermediate
aggregator feature. In the backend scheduler, M impalad will be selected
randomly as the intermediate aggregator for that runtime filter.
Information of this M selected impalad then passed from the scheduler to
coordinator as a RuntimeFilterAggregatorInfoPB. The coordinator then
converts the RuntimeFilterAggregatorInfoPB into a filter routing
information TRuntimeFilterAggDesc that is piggy-backed in
TRuntimeFilterSource.

A new RPC endpoint named UpdateFilterFromRemote is added in
data_stream_service.proto to handle filter updates from fellow impalad
executor to the designated aggregator impalad. This RPC will merge
filter updates into 'pending_remote_filter'. The intermediate aggregator
will then combine 'pending_remote_filter' with
'pending_merge_filter' (from local aggregation) into 'result_filter'
which is then sent to the coordinator. RuntimeFilterBank of the
intermediate aggregator will wait for all remote filter updates for at
least RUNTIME_FILTER_WAIT_TIME_MS. If RuntimeFilterBank is closing and
RUNTIME_FILTER_WAIT_TIME_MS has passed, any incomplete filter will be
marked as ALWAYS_TRUE and sent to the coordinator.

This patch currently targets the bloom filter produced by partitioned
join build only. Another kind of runtime filter is still efficient to
aggregate in coordinator only, while the bloom filter from broadcast
join only requires 1 valid filter update for publishing.

test_runtime_filters.py is modified to clarify the exec_options
dimension, test matrix constraints, and reduce pytest.skip() calls on
each test. runtime_filters.test is also changed to use counter
aggregation and assert on ExecSummary table so that they stay valid
irrespective of the number of fragment instances.

We benchmark the aggregation speed of 1 MB runtime filter aggregation on
20 executor nodes cluster with MT_DOP=36 that is instrumented to disable
local aggregation, simulating 720 runtime filter updates. The speed is
approximated as the duration between the earliest time a filter update
is made and the time that the coordinator publishes the complete filter.
The result is following:

+-++
| num aggregator node | Aggregation speed (ms) |
+-++
|   0 |   1296 |
|   1 |   1229 |
|   2 |608 |
|   4 |329 |
|   8 |205 |
+-++

Testing:
- Exercise MAX_NUM_FILTERS_AGGREGATED_PER_HOST in
  test_runtime_filters.py and query-options-test.cc
- Add custom_cluster/test_runtime_filter_aggregation.py.
- Pass exhaustive end-to-end and custom-cluster tests.

Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
---
M be/src/common/logging.h
M be/src/runtime/coordinator.cc
M be/src/runtime/data-stream-test.cc
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-filter-bank.h
M be/src/runtime/runtime-filter.cc
M be/src/runtime/runtime-filter.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/service/data-stream-service.cc
M be/src/service/data-stream-service.h
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/util/bloom-filter.cc
M be/src/util/bloom-filter.h
M be/src/util/network-util.h
M be/src/util/runtime-profile-counters.h
M common/protobuf/

[Impala-ASF-CR] IMPALA-12650: Skip some queries in test create unicode table on non-HFDS envs

2023-12-18 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20813 )

Change subject: IMPALA-12650: Skip some queries in test_create_unicode_table on 
non-HFDS envs
..


Patch Set 1: Code-Review+2

LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied4e5e295cf577cc45adc8d3b73d8682553d1b14
Gerrit-Change-Number: 20813
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 17:00:49 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12650: Skip some queries in test create unicode table on non-HFDS envs

2023-12-18 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20813 )

Change subject: IMPALA-12650: Skip some queries in test_create_unicode_table on 
non-HFDS envs
..


Patch Set 1: Verified+1 Code-Review+2

Merging this manually.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied4e5e295cf577cc45adc8d3b73d8682553d1b14
Gerrit-Change-Number: 20813
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 17:08:48 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12650: Skip some queries in test create unicode table on non-HFDS envs

2023-12-18 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20813 )

Change subject: IMPALA-12650: Skip some queries in test_create_unicode_table on 
non-HFDS envs
..

IMPALA-12650: Skip some queries in test_create_unicode_table on non-HFDS envs

Impala's non-HDFS test environment has no Hive running, making
these tests fail. Added IS_HDFS_ONLY section for queries that
need Hive.

Change-Id: Ied4e5e295cf577cc45adc8d3b73d8682553d1b14
Reviewed-on: http://gerrit.cloudera.org:8080/20813
Reviewed-by: Zoltan Borok-Nagy 
Reviewed-by: Csaba Ringhofer 
Tested-by: Csaba Ringhofer 
---
M testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test
1 file changed, 4 insertions(+), 0 deletions(-)

Approvals:
  Zoltan Borok-Nagy: Looks good to me, approved
  Csaba Ringhofer: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ied4e5e295cf577cc45adc8d3b73d8682553d1b14
Gerrit-Change-Number: 20813
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12650: Skip some queries in test create unicode table on non-HFDS envs

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20813 )

Change subject: IMPALA-12650: Skip some queries in test_create_unicode_table on 
non-HFDS envs
..


Patch Set 1:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied4e5e295cf577cc45adc8d3b73d8682553d1b14
Gerrit-Change-Number: 20813
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 17:17:15 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-12-18 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20612 )

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..


Patch Set 16: Code-Review+1

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20612/10/be/src/runtime/runtime-filter-bank.cc
File be/src/runtime/runtime-filter-bank.cc:

http://gerrit.cloudera.org:8080/#/c/20612/10/be/src/runtime/runtime-filter-bank.cc@139
PS10, Line 139: esc.filter_id);
> I think what you mean is, it is ok to allocate later as long as the whole t
Yes, this is the way I meant it, thanks


http://gerrit.cloudera.org:8080/#/c/20612/16/be/src/runtime/runtime-filter-bank.cc
File be/src/runtime/runtime-filter-bank.cc:

http://gerrit.cloudera.org:8080/#/c/20612/16/be/src/runtime/runtime-filter-bank.cc@241
PS16, Line 241: LOG(ERROR) << "Cannot allocate scratch bloom filter for 
pending_remote_filter "
  :<< "of filter " << params.filter_id();
  : bloom_filter = BloomFilter::ALWAYS_TRUE_FILTER;
Is my understanding right that we should fail the query in this case as this 
means a program error?

If failing the query from here is tricky, a DCHECK could be hit in DEBUG while 
RELEASE mode could rely on setting to ALWAYS_TRUE_FILTER. My concern with the 
graceful handling is that if there is an error like underestimating memory it 
may remain undiscovered in tests.


http://gerrit.cloudera.org:8080/#/c/20612/16/be/src/runtime/runtime-filter-bank.cc@291
PS16, Line 291: turn
nit: turned


http://gerrit.cloudera.org:8080/#/c/20612/16/be/src/runtime/runtime-filter-bank.cc@293
PS16, Line 293:   produced_filter.pending_remotes = 0;
Should we also set pending_producers to 0? It would be make sense for 
produced_filter.IsComplete() to return true below. Or it can be a problem if we 
won't wait for local filters?

Also, can you add a VLOG(2) line here to be able to trace this scenario?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
Gerrit-Change-Number: 20612
Gerrit-PatchSet: 16
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 18 Dec 2023 17:36:49 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20612 )

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..


Patch Set 16:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
Gerrit-Change-Number: 20612
Gerrit-PatchSet: 16
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 18 Dec 2023 17:48:56 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-12-18 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20612 )

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..


Patch Set 16:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20612/10/be/src/runtime/runtime-filter-bank.cc
File be/src/runtime/runtime-filter-bank.cc:

http://gerrit.cloudera.org:8080/#/c/20612/10/be/src/runtime/runtime-filter-bank.cc@722
PS10, Line 722: HECK_EQ(0, produced_filter
> It is closing only if query is completed, or canceled

It is not clear to what is the exact scenario when QueryState::Close is called.

A.it is enough for all fragment instances on the given impalad to finish 
execution, or
B. QueryState is kept alive as long as the query is running, even if this 
process has no tasks anymore.

In case of A, the content of the filters can still matter on fragment instances 
running in other processes. In case of B it seems wasteful to bother with 
sending filters  / waiting for not yet arrived remote filters.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
Gerrit-Change-Number: 20612
Gerrit-PatchSet: 16
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 18 Dec 2023 18:17:15 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-12-18 Thread Riza Suminto (Code Review)
Hello Kurt Deschler, Abhishek Rawat, Csaba Ringhofer, Michael Smith, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..

IMPALA-3825: Delegate runtime filter aggregation to some executors

IMPALA-4400 improve the runtime filter by aggregating runtime filters
locally before sending filter update to the coordinator and sharing a
single RuntimeFilterBank for all fragment instances in a query. However,
local filter aggregation is still insufficient if the number of nodes in
an impala cluster is large. For example, in a cluster of around 700
impalad backends, aggregation of 1 MB bloom filter updates in the
coordinator can exceed more than 1 second.

This patch aims to reduce coordinator load and speed up runtime filter
aggregation by doing intermediate aggregation in a few designated impala
backends before doing final aggregation and publishing in the
coordinator. Query option MAX_NUM_FILTERS_AGGREGATED_PER_HOST is added
to control this feature. Given N as the number of backend executors
excluding the coordinator, the selected number of intermediate
aggregators M = ceil(N / MAX_NUM_FILTERS_AGGREGATED_PER_HOST). Setting
MAX_NUM_FILTERS_AGGREGATED_PER_HOST <= 1 will disable the intermediate
aggregator feature. In the backend scheduler, M impalad will be selected
randomly as the intermediate aggregator for that runtime filter.
Information of this M selected impalad then passed from the scheduler to
coordinator as a RuntimeFilterAggregatorInfoPB. The coordinator then
converts the RuntimeFilterAggregatorInfoPB into a filter routing
information TRuntimeFilterAggDesc that is piggy-backed in
TRuntimeFilterSource.

A new RPC endpoint named UpdateFilterFromRemote is added in
data_stream_service.proto to handle filter updates from fellow impalad
executor to the designated aggregator impalad. This RPC will merge
filter updates into 'pending_remote_filter'. The intermediate aggregator
will then combine 'pending_remote_filter' with
'pending_merge_filter' (from local aggregation) into 'result_filter'
which is then sent to the coordinator. RuntimeFilterBank of the
intermediate aggregator will wait for all remote filter updates for at
least RUNTIME_FILTER_WAIT_TIME_MS. If RuntimeFilterBank is closing and
RUNTIME_FILTER_WAIT_TIME_MS has passed, any incomplete filter will be
marked as ALWAYS_TRUE and sent to the coordinator.

This patch currently targets the bloom filter produced by partitioned
join build only. Another kind of runtime filter is still efficient to
aggregate in coordinator only, while the bloom filter from broadcast
join only requires 1 valid filter update for publishing.

test_runtime_filters.py is modified to clarify the exec_options
dimension, test matrix constraints, and reduce pytest.skip() calls on
each test. runtime_filters.test is also changed to use counter
aggregation and assert on ExecSummary table so that they stay valid
irrespective of the number of fragment instances.

We benchmark the aggregation speed of 1 MB runtime filter aggregation on
20 executor nodes cluster with MT_DOP=36 that is instrumented to disable
local aggregation, simulating 720 runtime filter updates. The speed is
approximated as the duration between the earliest time a filter update
is made and the time that the coordinator publishes the complete filter.
The result is following:

+-++
| num aggregator node | Aggregation speed (ms) |
+-++
|   0 |   1296 |
|   1 |   1229 |
|   2 |608 |
|   4 |329 |
|   8 |205 |
+-++

Testing:
- Exercise MAX_NUM_FILTERS_AGGREGATED_PER_HOST in
  test_runtime_filters.py and query-options-test.cc
- Add custom_cluster/test_runtime_filter_aggregation.py.
- Pass exhaustive end-to-end and custom-cluster tests.

Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
---
M be/src/common/logging.h
M be/src/runtime/coordinator.cc
M be/src/runtime/data-stream-test.cc
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-filter-bank.h
M be/src/runtime/runtime-filter.cc
M be/src/runtime/runtime-filter.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/service/data-stream-service.cc
M be/src/service/data-stream-service.h
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/util/bloom-filter.cc
M be/src/util/bloom-filter.h
M be/src/util/network-util.h
M be/src/util/runtime-profile-counters.h
M common/protobuf/

[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-12-18 Thread Riza Suminto (Code Review)
Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20612 )

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..


Patch Set 17:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/20612/16/be/src/runtime/runtime-filter-bank.cc
File be/src/runtime/runtime-filter-bank.cc:

http://gerrit.cloudera.org:8080/#/c/20612/16/be/src/runtime/runtime-filter-bank.cc@241
PS16, Line 241: // but recover from it in RELEASE build by disabling 
filter
  : // (setting to ALWAYS_TRUE_FILTER).
  : DCHECK(false) << "Initial buffer for pending_re
> Is my understanding right that we should fail the query in this case as thi
Yes. Added DCHECK.


http://gerrit.cloudera.org:8080/#/c/20612/16/be/src/runtime/runtime-filter-bank.cc@291
PS16, Line 291:
> nit: turned
Done


http://gerrit.cloudera.org:8080/#/c/20612/16/be/src/runtime/runtime-filter-bank.cc@293
PS16, Line 293: --produced_filter.pending_remotes;
> Should we also set pending_producers to 0? It would be make sense for produ
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
Gerrit-Change-Number: 20612
Gerrit-PatchSet: 17
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 18 Dec 2023 18:30:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] WIP: Skip KRPC in local exchanges

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20810 )

Change subject: WIP: Skip KRPC in local exchanges
..


Patch Set 8: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10062/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee16cd943d76d04874bce7f3959e74b4685adb6e
Gerrit-Change-Number: 20810
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 18 Dec 2023 18:49:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-12-18 Thread Riza Suminto (Code Review)
Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20612 )

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..


Patch Set 17:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20612/10/be/src/runtime/runtime-filter-bank.cc
File be/src/runtime/runtime-filter-bank.cc:

http://gerrit.cloudera.org:8080/#/c/20612/10/be/src/runtime/runtime-filter-bank.cc@722
PS10, Line 722:
> > It is closing only if query is completed, or canceled
My assumption is A. I believe it is rare but not impossible. That is why I add 
logic to wait runtime filter arrival here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
Gerrit-Change-Number: 20612
Gerrit-PatchSet: 17
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 18 Dec 2023 18:51:24 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20612 )

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..


Patch Set 17:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
Gerrit-Change-Number: 20612
Gerrit-PatchSet: 17
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 18 Dec 2023 19:03:00 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12313: (part 3) Add UPDATE support for Iceberg tables.

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20760 )

Change subject: IMPALA-12313: (part 3) Add UPDATE support for Iceberg tables.
..

IMPALA-12313: (part 3) Add UPDATE support for Iceberg tables.

Part 2 had some limitations, most importantly it could not update
Iceberg tables if any of the following were true:
 * UPDATE value of partitioning column
 * UPDATE table that went through partition evolution
 * Table has SORT BY properties

The problem with partitions is that the delete record and new
data record might belong to different partitions and records
are shuffled across based on the partitions of the delete
records, hence the data files might not get written efficiently.

The problem with SORT BY properties, is that we need to write
the position delete files ordered by (file_path, position).

To address the above problems, this patch introduces a new
backend operator: IcebergBufferedDeleteSink. This new operator
extracts and aggregates the delete record information from
the incoming row batches, then in FlushFinal it orders the
position delete records and writes them out to files. This
mechanism is similar to Hive's approach:
https://github.com/apache/hive/pull/3251

IcebergBufferedDeleteSink cannot spill to disk, so it can only
run if there's enough memory to store the delete records. Paths
are stored only once, and the int64_t positions are stored in
a vector, so updating 100 Million records per node should require
around 800MBs + (100K) filepaths ~= 820 MBs of memory per node.
Spilling could be added later, but currently the need for it is not
too realistic.

Now records can get shuffled around based on the new data records'
partition values, and the SORT operator sorts the records based on
the SORT BY properties.

There's only one case we don't allow the UPDATE statement:
 * UPDATE partition column AND
 * Right-hand side of assignment is non-constant expression AND
 * UPDATE statement has a JOIN

When all of the above conditions meet, it would be possible to
have an incorrect JOIN condition that has multiple matches for the
data records, then the duplicated records would be shuffled
independently (based on the new partition value) to different
backend SINKs, and the different backend SINK would not be able
to detect the duplicates.

If any of the above conditions was false, then the duplicated records
would be shuffled together to the same SINK, that could do the
duplicate check.

This patch also moves some code from IcebergDeleteSink to the
newly introduced IcebergDeleteSinkBase.

Testing:
 * planner tests
 * e2e tests
 * Impala/Hive interop tests

Change-Id: I2bb97b4454165a292975d88dc9c23adb22ff7315
Reviewed-on: http://gerrit.cloudera.org:8080/20760
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
A be/src/exec/iceberg-buffered-delete-sink.cc
A be/src/exec/iceberg-buffered-delete-sink.h
A be/src/exec/iceberg-delete-sink-base.cc
A be/src/exec/iceberg-delete-sink-base.h
A be/src/exec/iceberg-delete-sink-config.cc
A be/src/exec/iceberg-delete-sink-config.h
M be/src/exec/iceberg-delete-sink.cc
M be/src/exec/iceberg-delete-sink.h
M be/src/exec/table-sink-base.cc
M be/src/exec/table-sink-base.h
M be/src/exprs/slot-ref.h
M be/src/runtime/dml-exec-state.h
M common/thrift/DataSinks.thrift
M fe/src/main/java/org/apache/impala/analysis/DmlStatementBase.java
M fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyImpl.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/OptimizeStmt.java
A fe/src/main/java/org/apache/impala/planner/IcebergBufferedDeleteSink.java
M fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M testdata/datasets/functional/functional_schema_template.sql
M 
testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-update.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-update-basic.test
A 
testdata/workloads/functional-query/queries/QueryTest/iceberg-update-partitions.test
A 
testdata/workloads/functional-query/queries/QueryTest/iceberg-update-stress.test
M tests/query_test/test_iceberg.py
M tests/stress/test_update_stress.py
35 files changed, 1,951 insertions(+), 349 deletions(-)

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

--
To view, visit http://gerrit.cloudera.org:8080/20760
To unsubscribe, visit http://gerrit.cloudera.org:8080/setting

[Impala-ASF-CR] IMPALA-12313: (part 3) Add UPDATE support for Iceberg tables.

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20760 )

Change subject: IMPALA-12313: (part 3) Add UPDATE support for Iceberg tables.
..


Patch Set 13: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2bb97b4454165a292975d88dc9c23adb22ff7315
Gerrit-Change-Number: 20760
Gerrit-PatchSet: 13
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 19:14:20 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10976: Sync db/table to latest HMS event for all DDL/DMLs

2023-12-18 Thread Sai Hemanth Gantasala (Code Review)
Hello Quanlong Huang, k.venureddy2...@gmail.com, Csaba Ringhofer, Impala Public 
Jenkins,

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

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

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

Change subject: IMPALA-10976: Sync db/table to latest HMS event for all DDL/DMLs
..

IMPALA-10976: Sync db/table to latest HMS event for all DDL/DMLs

The idea is that when any DDL/DML operation is performed by Impala, it
also syncs the db/table to its latest event ID as per HMS. This way
updates to a db/table's are applied in the same order as they appear in
the Notification log table in HMS which ensures consistency. Currently
catalogD applies any updates received from Impala clients in-place.
Instead it should perform an HMS operation first and then replay all
the HMS events since the last synced event id.

Implementation: when the enable_sync_to_latest_event_on_ddls flag is
set to true, we do the DDL/DML operation first, i.e., perform HMS
operation and then sync the db/table in the catalogD's cache to the
latest event in HMS for the corresponding db/table. Currently we fetch
all events greater than the db/table's lastSyncEventId and filter them
in the events processor to sync only the current db/table events. Once
HIVE-27499 is implemented, we can directly fetch the events only for
the respective db/table and process them. Currently, there is no
efficient way to identify if there are pending events for a db/table.

Set 'enable_sync_to_latest_event_on_ddls' to true.

Note: We don't modify the cache using MetastoreEventsProcessor for
alter table rename operation as this is a complex operation regarding
cache modification (IMPALA-12553 has more details about this). We also
don't modify the cache this way for the truncate table operation
(IMPALA-12636 has more details about this). We don't modify cache using
above process for 'refresh table'/'invalidate metadata table' commands.

Testing:
1) Added few tests in the MetaStoreEventProcessorForTest to verify this
feature that simulates the metadata sync between HMS and Impala.
2) Added few tests in the CatalogHmsSyncToLatestEventIdTest class to
the metadata sync between HMS end point, Catalog Metastore Server and
Impala. The HMS end point serves as common interface to metadata
changes outside the current Impala service such as Hive, Spark or other
Impala service. Also verified the table lastSyncEventId is updated
after the events are sync and confirmed that metastore event processor
ignored these synced events.
3) Added some end-to-end tests in test_sync_to_latest_hms_events.py

Change-Id: Ia250d0a943838086c187e5cb7c60035e5a564bbf
---
M be/src/catalog/catalog-server.cc
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M 
fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M 
fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
M 
fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsSyncToLatestEventIdTest.java
A tests/custom_cluster/test_sync_to_latest_hms_events.py
M tests/metadata/test_recover_partitions.py
14 files changed, 1,250 insertions(+), 132 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/20367/21
--
To view, visit http://gerrit.cloudera.org:8080/20367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia250d0a943838086c187e5cb7c60035e5a564bbf
Gerrit-Change-Number: 20367
Gerrit-PatchSet: 21
Gerrit-Owner: Sai Hemanth Gantasala 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Sai Hemanth Gantasala 


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-12-18 Thread Riza Suminto (Code Review)
Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20612 )

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..


Patch Set 17:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20612/10/be/src/runtime/runtime-filter-bank.cc
File be/src/runtime/runtime-filter-bank.cc:

http://gerrit.cloudera.org:8080/#/c/20612/10/be/src/runtime/runtime-filter-bank.cc@722
PS10, Line 722:
> My assumption is A. I believe it is rare but not impossible. That is why I
Runtime filter is most needed in big scan fragment, which most likely 
distributed across all executor nodes, including nodes selected as intermediate 
aggregator. If an executor still has running scan, then its RuntimeFilterBank 
will stay open.

If an executor does not have any fragment instance running anymore, then its 
RuntimeFilterBank may be closing. If this is true for all executors, then 
pending runtime filter does not matter anymore since query is completing anyway.

I can think a corner case where intermediate aggregator is also working on the 
big scan fragment, but only scheduled to handle fewer scan ranges than other 
nodes. Thus, it completes faster than other executor nodes.

In any case, missing filter should not impact query correctness. Just its 
overall performance.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
Gerrit-Change-Number: 20612
Gerrit-PatchSet: 17
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 18 Dec 2023 19:22:42 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight

2023-12-18 Thread Michael Smith (Code Review)
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
..

IMPALA-12633: Remove DCHECK for slow SetQueryInflight

Removes the DCHECK that the original query is inflight before trying to
close it during a query retry. SetQueryInflight is a separate operation
the server performs after a query has started executing async, and it's
possible for the query to fail and retry before the server calls
SetQueryInflight. When that happens, we still need to perform cleanup
or the original request_state is never closed and we hit a different
DCHECK: "BlockOnWait() needs to be called!"

Updates the message from DCHECK in ClientRequestState's destructor to
reflect that wait_thread_ is only reset in Finalize.

Adds a debug action and test where just the original query is delayed
during the SetQueryInflight call.

Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
---
M be/src/runtime/query-driver.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/custom_cluster/test_query_retries.py
5 files changed, 91 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight

2023-12-18 Thread Michael Smith (Code Review)
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
..

IMPALA-12633: Remove DCHECK for slow SetQueryInflight

Removes the DCHECK that the original query is inflight before trying to
close it during a query retry. SetQueryInflight is a separate operation
the server performs after a query has started executing async, and it's
possible for the query to fail and retry before the server calls
SetQueryInflight. When that happens, we still need to perform cleanup
or the original request_state is never closed and we hit a different
DCHECK: "BlockOnWait() needs to be called!"

Updates the message from DCHECK in ClientRequestState's destructor to
reflect that wait_thread_ is only reset in Finalize.

Adds a debug action and test where just the original query is delayed
during the SetQueryInflight call.

Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
---
M be/src/runtime/query-driver.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/custom_cluster/test_query_retries.py
5 files changed, 92 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight

2023-12-18 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20799 )

Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
..


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/client-request-state.cc@179
PS5, Line 179: Finalize() ne
> This error message is stale and misleading. This line was added in commit b
Done


http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.h@764
PS5, Line 764:   /// in-flight until the query is unregistered. Until a query 
is in-flight, an attempt
> It'd be nice to also mention prestopped_queries.
Done


http://gerrit.cloudera.org:8080/#/c/20799/4/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/20799/4/be/src/service/impala-server.cc@1615
PS4, Line 1615:   query_handle->UnRegisterRemainingRPCs();
> We can check whether the insertion took place and add a warning log if it d
Done


http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.cc@1616
PS5, Line 1616:
> Could you add a comment here for why we bump the counter when closing a que
I rearranged SetQueryInflight a little bit to handle incrementing it; 
conceptually it makes more sense that it's still handled when SetQueryInflight 
is called.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 18 Dec 2023 19:35:14 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-10976: Sync db/table to latest HMS event for all DDL/DMLs

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20367 )

Change subject: IMPALA-10976: Sync db/table to latest HMS event for all DDL/DMLs
..


Patch Set 21:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia250d0a943838086c187e5cb7c60035e5a564bbf
Gerrit-Change-Number: 20367
Gerrit-PatchSet: 21
Gerrit-Owner: Sai Hemanth Gantasala 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Mon, 18 Dec 2023 19:41:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12606: Sporadic failures around query test.test queries.TestQueries.test intersect

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20793 )

Change subject: IMPALA-12606: Sporadic failures around 
query_test.test_queries.TestQueries.test_intersect
..


Patch Set 6: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10064/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b855c59808db80fd7ac596ce338fc4c3c9c7667
Gerrit-Change-Number: 20793
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 19:47:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20799 )

Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
..


Patch Set 6:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 18 Dec 2023 19:57:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight

2023-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20799 )

Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
..


Patch Set 7:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 18 Dec 2023 19:57:55 +
Gerrit-HasComments: No


  1   2   >