Skip to content

Improvement #8161 : Cardinality estimation should use primary record versions only#8162

Merged
hvlad merged 2 commits intoB3_0_Releasefrom
work/gh-8161
Jun 18, 2024
Merged

Improvement #8161 : Cardinality estimation should use primary record versions only#8162
hvlad merged 2 commits intoB3_0_Releasefrom
work/gh-8161

Conversation

@hvlad
Copy link
Copy Markdown
Member

@hvlad hvlad commented Jun 18, 2024

No description provided.

@hvlad hvlad self-assigned this Jun 18, 2024
@hvlad hvlad requested a review from dyemanov June 18, 2024 12:02
@aafemt
Copy link
Copy Markdown
Contributor

aafemt commented Jun 18, 2024

Do I understand right that your code counts records on only one data page because done become true as soon as recordCount get any non-zero value and inner loop is aborted as well? Old code seems to count records for whole table...

@hvlad
Copy link
Copy Markdown
Member Author

hvlad commented Jun 18, 2024

Do I understand right that your code counts records on only one data page because done become true as soon as recordCount get any non-zero value and inner loop is aborted as well?

Almost correct

Old code seems to count records for whole table...

No, read it again

Comment thread src/jrd/dpm.epp Outdated
const RelationPages* const relPages = relation->getPages(tdbb);
RelationPages* const relPages = relation->getPages(tdbb);
const vcl* const vector = relPages->rel_pages;
if (vector)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like vector is not used anymore, so maybe just if (relPages->rel_pages) ?
Not sure this check is needed at all, BTW.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with vector, as for the check - it makes no harm.

@hvlad hvlad merged commit daac348 into B3_0_Release Jun 18, 2024
@hvlad hvlad linked an issue Jun 18, 2024 that may be closed by this pull request
@hvlad hvlad deleted the work/gh-8161 branch September 20, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cardinality estimation should use primary record versions only

3 participants