You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Jason Fehr (Code Review)" <ge...@cloudera.org> on 2025/09/02 18:18:55 UTC

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

Jason Fehr has uploaded this change for review. ( http://gerrit.cloudera.org:8080/23365


Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................

IMPALA-14382: Fix Null Pointer Dereference

Fixes a potential null pointer dereference when log level >= 2.

Adds 'build' as a valid EE test helper directory as VSCode creates
this directory.

Tested locally by running test_scanners from the query_test EE test
suite using a release build of Impala and log level 2. Minidumps were
not generated during this test run but were generated during the same
test run without this fix applied.

Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
---
M be/src/exec/base-sequence-scanner.cc
M tests/run-tests.py
2 files changed, 3 insertions(+), 2 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 1
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/23365/2/be/src/exec/base-sequence-scanner.cc
File be/src/exec/base-sequence-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/23365/2/be/src/exec/base-sequence-scanner.cc@125
PS2, Line 125:   VLOG_FILE << "Bytes read past scan range: " << (stream_ == nullptr ? -1 :
             :       -stream_->bytes_left());
> I meant, HdfsScanner::CloseInternal()
I see nullptr check in HdfsTextScanner::Close() and HdfsTextScanner::Close(). So it is definitely possible to have stream_ == nullptr at Close().

Maybe do the same nullptr check here and log differently?

  VLOG_FILE << "stream_ is not initialized yet";



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 2
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 18:56:07 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................

IMPALA-14382: Fix Null Pointer Dereference

Fixes a potential null pointer dereference when log level >= 2.

Adds 'build' as a valid EE test helper directory as VSCode creates
this directory.

Tested locally by running test_scanners from the query_test EE test
suite using a release build of Impala and log level 2. Minidumps were
not generated during this test run but were generated during the same
test run without this fix applied.

Generated-by: Github Copilot (Claude Sonnet 3.7)
Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
---
M be/src/exec/base-sequence-scanner.cc
M tests/run-tests.py
2 files changed, 3 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 3
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/23365/2/be/src/exec/base-sequence-scanner.cc
File be/src/exec/base-sequence-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/23365/2/be/src/exec/base-sequence-scanner.cc@125
PS2, Line 125:   VLOG_FILE << "Bytes read past scan range: " << (stream_ == nullptr ? 0 :
             :       -stream_->bytes_left());
> I'm not sure exactly why stream_ is nullptr.  Since this is a log level 2 m
Done


http://gerrit.cloudera.org:8080/#/c/23365/2/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/23365/2/tests/run-tests.py@48
PS2, Line 48: build
> I've been using VSCode and never see this created. It also not checked into
I still think this should be dropped if the dir is not checked in to the repo.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 3
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 19:30:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/23365/2/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/23365/2/tests/run-tests.py@48
PS2, Line 48: build
> Oh interesting.  When I remove the tests/build directory and reload VSCode,
Clarification:  when the build directory exists, EE tests fail because of this code:  https://github.com/apache/impala/blob/843de44788758ab00694bc456e0d92608a8a4c60/tests/run-tests.py#L218-L220



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 19:37:29 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/23365/2/be/src/exec/base-sequence-scanner.cc
File be/src/exec/base-sequence-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/23365/2/be/src/exec/base-sequence-scanner.cc@125
PS2, Line 125:   VLOG_FILE << "Bytes read past scan range: " << (stream_ == nullptr ? 0 :
             :       -stream_->bytes_left());
> I see nullptr check in HdfsTextScanner::Close() and HdfsTextScanner::Close(
I'm not sure exactly why stream_ is nullptr.  Since this is a log level 2 message in existing code, I don't want to deviate from the original intent of this log message and don't want to add any new log messages in case there is code some where relying on the "Bytes read past scan range" and "Average block size" messages being one after the other.

I did realize that a default of -1 is incorrect.  The bytes read past scan is always reported as a negative number.  Thus, 144 bytes read will be reported as -144.  This means -1 is reporting that a single byte was read past the scan range which is incorrect.  I updated the log message to use 0 as the default since an unopened stream cannot have read any bytes past its end.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 3
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 19:25:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 Sep 2025 01:14:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 1
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 18:40:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 3
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 19:53:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/23365 )

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................

IMPALA-14382: Fix Null Pointer Dereference

Fixes a potential null pointer dereference when log level >= 2.

Adds 'build' as a valid EE test helper directory as VSCode creates
this directory.

Tested locally by running test_scanners from the query_test EE test
suite using a release build of Impala and log level 2. Minidumps were
not generated during this test run but were generated during the same
test run without this fix applied.

Generated-by: Github Copilot (Claude Sonnet 3.7)
Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
---
M be/src/exec/base-sequence-scanner.cc
M tests/run-tests.py
2 files changed, 3 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 2
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/23365/2/be/src/exec/base-sequence-scanner.cc
File be/src/exec/base-sequence-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/23365/2/be/src/exec/base-sequence-scanner.cc@125
PS2, Line 125:   VLOG_FILE << "Bytes read past scan range: " << (stream_ == nullptr ? -1 :
             :       -stream_->bytes_left());
> Can you turn on the log level 2 and find out why stream_ is nullptr?
I meant, HdfsScanner::CloseInternal()



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 2
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 18:41:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/23365 )

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................

IMPALA-14382: Fix Null Pointer Dereference

Fixes a potential null pointer dereference when log level >= 2.

Adds 'build' as a valid EE test helper directory as VSCode creates
this directory.

Tested locally by running test_scanners from the query_test EE test
suite using a release build of Impala and log level 2. Minidumps were
not generated during this test run but were generated during the same
test run without this fix applied.

Generated-by: Github Copilot (Claude Sonnet 3.7)
Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Reviewed-on: http://gerrit.cloudera.org:8080/23365
Reviewed-by: Riza Suminto <ri...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/base-sequence-scanner.cc
M tests/.gitignore
M tests/run-tests.py
3 files changed, 4 insertions(+), 2 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/23365/2/be/src/exec/base-sequence-scanner.cc
File be/src/exec/base-sequence-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/23365/2/be/src/exec/base-sequence-scanner.cc@125
PS2, Line 125:   VLOG_FILE << "Bytes read past scan range: " << (stream_ == nullptr ? -1 :
             :       -stream_->bytes_left());
Can you turn on the log level 2 and find out why stream_ is nullptr?

Open should initialize it
https://github.com/apache/impala/blob/843de44788758ab00694bc456e0d92608a8a4c60/be/src/exec/hdfs-scanner.cc#L98
There are only two other callsite that write to stream_: BaseSequenceScanner::Close() and HdfsParquetScanner::Open().

Does it mean there is a sequence scanner that Close() before Open()?


http://gerrit.cloudera.org:8080/#/c/23365/2/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/23365/2/tests/run-tests.py@48
PS2, Line 48: build
I've been using VSCode and never see this created. It also not checked into Apache Impala repo.
Can this be removed / ignored?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 2
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 18:38:27 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 2
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 18:29:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/23365/2/be/src/exec/base-sequence-scanner.cc
File be/src/exec/base-sequence-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/23365/2/be/src/exec/base-sequence-scanner.cc@125
PS2, Line 125:   VLOG_FILE << "Bytes read past scan range: " << (stream_ == nullptr ? -1 :
             :       -stream_->bytes_left());
> I see nullptr check in HdfsTextScanner::Close() and HdfsTextScanner::Close(
HdfsJsonScanner::Close()



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 2
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 19:17:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 4:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 20:01:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/23365/2/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/23365/2/tests/run-tests.py@48
PS2, Line 48: build
> I've been using VSCode and never see this created. It also not checked into
Oh interesting.  When I remove the tests/build directory and reload VSCode, the directory is re-created.  It may be because I have "tests" as a top-level directory in my workspace.

I would like to leave this as running EE tests fails each time for me.  I updated tests/.gitignore to exclude the entire build directory.  There is already an entry that seems to have been an attempt to exclude the build directory:  https://github.com/apache/impala/blob/843de44788758ab00694bc456e0d92608a8a4c60/tests/.gitignore#L2



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 19:34:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................

IMPALA-14382: Fix Null Pointer Dereference

Fixes a potential null pointer dereference when log level >= 2.

Adds 'build' as a valid EE test helper directory as VSCode creates
this directory.

Tested locally by running test_scanners from the query_test EE test
suite using a release build of Impala and log level 2. Minidumps were
not generated during this test run but were generated during the same
test run without this fix applied.

Generated-by: Github Copilot (Claude Sonnet 3.7)
Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
---
M be/src/exec/base-sequence-scanner.cc
M tests/.gitignore
M tests/run-tests.py
3 files changed, 4 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 4: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/23365/2/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/23365/2/tests/run-tests.py@48
PS2, Line 48: build
> Oh interesting.  When I remove the tests/build directory and reload VSCode,
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 19:37:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14382: Fix Null Pointer Dereference

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

Change subject: IMPALA-14382: Fix Null Pointer Dereference
......................................................................


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91660aa84407c17ffb7cd3c721d4f3f0a844d61d
Gerrit-Change-Number: 23365
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Sep 2025 19:49:38 +0000
Gerrit-HasComments: No