Relax validation for empty pages in PageValidations #27569

losipiuk · 2025-12-07T15:06:05Z

Description

Allow empty pages without blocks. It is valid for connectors/table
functions to produce empty pages with empty block list interleaved with
pages with data. Empty pages are filtered out by Driver.

Additional context and related issues

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix some things. ({issue}`issuenumber`)

raunaqmorarka · 2025-12-08T05:34:50Z

core/trino-main/src/main/java/io/trino/split/PageValidations.java


    public static void validateOutputPageTypes(SourcePage page, List<Type> expectedTypes, Supplier<String> debugContextSupplier)
    {
+        if (page.getPositionCount() == 0 && page.getChannelCount() == 0) {


I would drop && page.getChannelCount() == 0
It is valid to create an empty Page as new Page(0, new Block[N]), but that would throw a nullptr exception in the current code.
We should probably tighten up the Page constructor to disallow that.

It is valid to create an empty Page as new Page(0, new Block[N]), but that would throw a nullptr exception in the current code.

how would that be a valid page?
The blocks are not nullable in the page

We should probably tighten up the Page constructor to disallow that.

it doesn't have to be a runtime check (might be redundant in hot path)
the fact there is no such check should not be construed as implying that null blocks are OK

findepi · 2025-12-08T09:04:16Z

core/trino-main/src/main/java/io/trino/split/PageValidations.java

    public static void validateOutputPageTypes(SourcePage page, List<Type> expectedTypes, Supplier<String> debugContextSupplier)
    {
+        if (page.getPositionCount() == 0 && page.getChannelCount() == 0) {
+            // allow empty pages without blocks


The comment says what we do, but doesn't say why. Please update & update PR description accordingly.

Also, it's not a universal truth that empty page can be any shape. For example

trino/core/trino-main/src/main/java/io/trino/server/protocol/JsonEncodingUtils.java

Lines 121 to 128 in 26846cb

verify(typeEncoders.length == sourcePageChannels.length, "Source page channels and type encoders must have the same length");

try {

generator.writeStartArray();

for (Page page : pages) {

Block[] blocks = new Block[sourcePageChannels.length];

for (int i = 0; i < sourcePageChannels.length; i++) {

blocks[i] = page.getBlock(sourcePageChannels[i]);

This may not break because empty pages are filtered somewhere earlier.

The comment says what we do, but doesn't say why. Please update & update PR description accordingly.

That is a good point. I do not know any concrete place in code which does produce such pages. It is based on hint from @raunaqmorarka that it is valid scenario, so I sent this PR defensively.
@raunaqmorarka do you maybe have specific code pointers?

I do not know any concrete place in code which does produce such pages.

example

trino/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/functions/tablechanges/TableChangesFunctionProcessor.java

Line 137 in 51087fb

return TableFunctionProcessorState.Processed.produced(EMPTY_PAGE);

Allow empty pages without blocks. It is valid for connectors/table functions to produce empty pages with empty block list interleaved with pages with data. Empty pages are filtered out by Driver.

findepi · 2025-12-08T13:53:57Z

core/trino-main/src/main/java/io/trino/split/PageValidations.java

    public static void validateOutputPageTypes(SourcePage page, List<Type> expectedTypes, Supplier<String> debugContextSupplier)
    {
+        if (page.getPositionCount() == 0 && page.getChannelCount() == 0) {
+            // Allow empty pages without blocks. It is valid for connectors/table functions to produce empty pages with empty block list


per

trino/core/trino-main/src/main/java/io/trino/operator/Driver.java

Line 407 in 993a088

if (page != null && page.getPositionCount() != 0) {

drop page.getChannelCount() == 0 condition and update the comment

I understood from the discussion that such pages are really broken and not created? Are we creating such anywhere. Or just drop just in case?

i don't know of any code creating empty pages with zero position count and non-zero, but incorrect, channel count

Or just drop just in case?

this. since the Driver condition is what it and same is in Scan Filter operator

trino/core/trino-main/src/main/java/io/trino/operator/project/PageProcessor.java

Lines 115 to 116 in 5bdd75d

if (page.getPositionCount() == 0) {

return WorkProcessor.of();

findepi · 2025-12-08T13:54:26Z

core/trino-main/src/main/java/io/trino/split/PageValidations.java

    {
+        if (page.getPositionCount() == 0 && page.getChannelCount() == 0) {
+            // Allow empty pages without blocks. It is valid for connectors/table functions to produce empty pages with empty block list
+            // interleaved with pages with data. Example of such behavior can be found in TableChangesFunctionProcessor (this is not exactly


Suggested change

// interleaved with pages with data. Example of such behavior can be found in TableChangesFunctionProcessor (this is not exactly

// interleaved with pages with data. Example of such behavior can be found in Delta's TableChangesFunctionProcessor (this is not exactly

findepi · 2025-12-08T13:58:09Z

core/trino-main/src/main/java/io/trino/split/PageValidations.java

+        if (page.getPositionCount() == 0 && page.getChannelCount() == 0) {
+            // Allow empty pages without blocks. It is valid for connectors/table functions to produce empty pages with empty block list
+            // interleaved with pages with data. Example of such behavior can be found in TableChangesFunctionProcessor (this is not exactly
+            // related to SourcePage, but illustrates the contract, which is the same for Pages and SourcePages).


I realized that for a SourcePage, the check may be triggering a load of lazy data.
If we had applied the check outside of ScanFilterAndProjectOperator, this wouldn't be a problem (and we wouldn't see SourcePage), but now maybe it is?

losipiuk · 2025-12-08T19:45:13Z

Incorporated into #27582

cla-bot bot added the cla-signed label Dec 7, 2025

losipiuk requested review from Copilot, findepi and raunaqmorarka and removed request for Copilot December 7, 2025 15:06

Copilot started reviewing on behalf of losipiuk December 7, 2025 15:06 View session

raunaqmorarka reviewed Dec 8, 2025

View reviewed changes

findepi reviewed Dec 8, 2025

View reviewed changes

Relax validation for empty pages in PageValidations

7850ace

Allow empty pages without blocks. It is valid for connectors/table functions to produce empty pages with empty block list interleaved with pages with data. Empty pages are filtered out by Driver.

losipiuk force-pushed the lukaszos/relax-validation-for-empty-pages-7e2694 branch from 6fe187a to 7850ace Compare December 8, 2025 13:52

findepi approved these changes Dec 8, 2025

View reviewed changes

losipiuk closed this Dec 8, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Relax validation for empty pages in PageValidations #27569

Relax validation for empty pages in PageValidations #27569

Uh oh!

losipiuk commented Dec 7, 2025 •

edited

Loading

Uh oh!

raunaqmorarka Dec 8, 2025

Uh oh!

findepi Dec 8, 2025

Uh oh!

findepi Dec 8, 2025

Uh oh!

losipiuk Dec 8, 2025

Uh oh!

findepi Dec 8, 2025

Uh oh!

losipiuk Dec 8, 2025

Uh oh!

findepi Dec 8, 2025

Uh oh!

losipiuk Dec 8, 2025

Uh oh!

findepi Dec 8, 2025

Uh oh!

findepi Dec 8, 2025

Uh oh!

findepi Dec 8, 2025

Uh oh!

losipiuk commented Dec 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

	verify(typeEncoders.length == sourcePageChannels.length, "Source page channels and type encoders must have the same length");
	try {
	generator.writeStartArray();

	for (Page page : pages) {
	Block[] blocks = new Block[sourcePageChannels.length];
	for (int i = 0; i < sourcePageChannels.length; i++) {
	blocks[i] = page.getBlock(sourcePageChannels[i]);

	if (page.getPositionCount() == 0) {
	return WorkProcessor.of();

	// interleaved with pages with data. Example of such behavior can be found in TableChangesFunctionProcessor (this is not exactly
	// interleaved with pages with data. Example of such behavior can be found in Delta's TableChangesFunctionProcessor (this is not exactly

Relax validation for empty pages in PageValidations #27569

Relax validation for empty pages in PageValidations #27569

Uh oh!

Conversation

losipiuk commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Additional context and related issues

Release notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

losipiuk commented Dec 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

losipiuk commented Dec 7, 2025 •

edited

Loading