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

Upgrade Pinot libraries to 0.11.0 #14090

Merged
merged 3 commits into from
Oct 8, 2022
Merged

Conversation

elonazoulay
Copy link
Member

Description

Non-technical explanation

Release notes

( ) This is not user-visible 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`)

plugin/trino-pinot/pom.xml Outdated Show resolved Hide resolved
<groupId>org.basepom.maven</groupId>
<artifactId>duplicate-finder-maven-plugin</artifactId>
<configuration>
<ignoredDependencies>
Copy link
Member

Choose a reason for hiding this comment

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

What library is contributing the duplicates? Can we improve this by shading upstream?

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw it one due to the recent helix-core upgrade to 1.0.4. It brings several helix-* and they become duplicate each other, which I don't know the reason

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw the warning of duplicate same version. So I feel it's safe to ignore

// Add a null row, verify it was not ingested as pinot does not accept null time column values.
// The data is verified in testBrokerQueryWithTooManyRowsForSegmentQuery
tooManyRowsRecordsBuilder.add(new ProducerRecord<>(TOO_MANY_ROWS_TABLE, "key" + MAX_ROWS_PER_SPLIT_FOR_SEGMENT_QUERIES, new GenericRecordBuilder(tooManyRowsAvroSchema).build()));
//tooManyRowsRecordsBuilder.add(new ProducerRecord<>(TOO_MANY_ROWS_TABLE, "key" + MAX_ROWS_PER_SPLIT_FOR_SEGMENT_QUERIES, new GenericRecordBuilder(tooManyRowsAvroSchema).build()));
Copy link
Member

Choose a reason for hiding this comment

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

Probably extract this specific case to new test method and override it in <0.11?

@@ -139,11 +139,12 @@ public void testDoubleWithScientificNotation()
@Test
public void testFilterWithCast()
{
// UPDATE: Pinot 0.11.0 automatically casts constants during parsing
Copy link
Member

Choose a reason for hiding this comment

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

can be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe it's safer to keep it, to verify the behavior? Or did you mean just remove the comment?

Copy link
Member

Choose a reason for hiding this comment

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

I meant the comment.

TestPinotWithoutAuthenticationIntegrationLatestVersionConnectorSmokeTest
had a typo in the name.
public static final String PINOT_LATEST_IMAGE_NAME = "apachepinot/pinot:0.10.0";
public static final String PINOT_PREVIOUS_IMAGE_NAME = "apachepinot/pinot:0.9.3-jdk11";
public static final String PINOT_LATEST_IMAGE_NAME = "apachepinot/pinot:0.11.0";
public static final String PINOT_PREVIOUS_IMAGE_NAME = "apachepinot/pinot:0.10.0";
Copy link
Member

Choose a reason for hiding this comment

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

lower bound of supported versions is now 0.10? Please update pinot.rst too accordingly.

@bagipriyank
Copy link

any updates on this pr?

@github-actions github-actions bot added the docs label Oct 8, 2022
@martint martint merged commit 0374342 into trinodb:master Oct 8, 2022
@github-actions github-actions bot added this to the 400 milestone Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants