Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for failure of unsigned_joins test under centos8 #1822

Conversation

mariadb-SergeyZefirov
Copy link
Contributor

The problem was that column width was not set during BRM reporting stage in cpimport processing. That made column width a garbage, prevented correct ranges to be set and as a result some extents were incorrectly skipped during SELECT operation.

The fixes are in we_brmupdater.cpp: a change to display proper "CP" line format in the comments in the BRM report file, a change to add print of column width into file and a change to parse column width.

Debug logs

Debug logs - fix typo

Debug logs

Debug logs

debug logs

debug logs

debug logs

debug logs

debug logs

debug logs

debug logs

debug logs

Solution to test

Remove debug logs
@drrtuy drrtuy merged commit e61f6cb into mariadb-corporation:develop Mar 29, 2021
throw (runtime_error("Bad column width in CP entry string"));
}
pTok = strtok(NULL, " ");

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to add support for 128-bit min/max values as well here. The sending side BRMReporter::sendCPToFile either streams CPInfoMerge::max/CPInfoMerge::min or CPInfoMerge::bigMax/CPInfoMerge::bigMin based on CPInfoMerge::type and CPInfoMerge::colWidth. So the parser here should first parse out type and colWidth and then based on these 2 values, should either set the 128-bit or the 64-bit min/max values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

It is not possible to have tests for that functionality using joins. I've created task specifically to address your comment: https://jira.mariadb.org/browse/MCOL-4652

@tntnatbry
Copy link
Contributor

Looks like the PR is already merged before my review. I have requested changes in the comment above.

Commit message is also not correct.

I would also add the MCOL number as the first thing in the PR title.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants