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

Portable with OBJECT format on SQL when factory is missing #18891

Merged
merged 3 commits into from
Jun 17, 2021

Conversation

sancar
Copy link
Contributor

@sancar sancar commented Jun 11, 2021

When the factory config is missing on the server but
the map is configured with in-memory format Object, we can store
Portables as PortableGenericRecord and still query them
without needing to convert them to Object/Data.

This PR adds related test to old query engine.
Also updates new query system to allow that usage and adds
a related test.

Checklist:

  • Labels (Team:, Type:, Source:, Module:) and Milestone set
  • Add Add to Release Notes label if changes should be mentioned in release notes or Not Release Notes content if changes are not relevant for release notes
  • Request reviewers if possible
  • New public APIs have @Nonnull/@Nullable annotations
  • New public APIs have @since tags in Javadoc
  • Send backports/forwardports if fix needs to be applied to past/future releases

@sancar sancar added Type: Enhancement Team: Client Source: Internal PR or issue was opened by an employee labels Jun 11, 2021
@sancar sancar added this to the 5.0 milestone Jun 11, 2021
@sancar sancar self-assigned this Jun 11, 2021
@sancar sancar requested a review from a team as a code owner June 11, 2021 13:00
@sancar sancar changed the title More usable Portable when factory is missing on the cluster Portable with OBJECT format on SQL when factory is missing Jun 15, 2021
When the factory config is missing on the server but
server is configured in memory format as Data, we can store
Portable's as PortableGenericRecord and still query them
without needing to convert them to Object/Data.

This pr adds related test to old query.
Also updates new query system to allow that usage and adds
a related test.
@sancar sancar force-pushed the fix/portableNewSql/master branch from a495a88 to f47d01c Compare June 15, 2021 12:54

SqlResult rows = client.getSql().execute("SELECT * FROM test WHERE i >= 45");
AtomicInteger integer = new AtomicInteger(0);
rows.iterator().forEachRemaining(sqlRow -> integer.incrementAndGet());
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be simplified with Iterators.size().


Iterator<SqlRow> iterator = rows.iterator();
SqlRow row = iterator.next();
assertFalse(iterator.hasNext());
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be simplified with Iteratros.getOnlyElement().

@gierlachg
Copy link
Contributor

gierlachg commented Jun 16, 2021

I would wait for @viliam-durina's review as well.

@hazelcast hazelcast deleted a comment from hz-devops-test Jun 16, 2021
@sancar
Copy link
Contributor Author

sancar commented Jun 16, 2021

@gierlachg @viliam-durina Thanks for the reviews. I will not backport this to older maintenance branches since the feature was Beta, if that is ok ?

@hz-devops-test
Copy link

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file
--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO] 
[ERROR] Errors: 
[ERROR]   ClientExecutorSplitBrainProtectionReadTest.setUp:44->AbstractSplitBrainProtectionTest.initTestEnvironment:105->AbstractSplitBrainProtectionTest.initCluster:247 ? SplitBrainProtection
[INFO] 
[ERROR] Tests run: 38557, Failures: 0, Errors: 1, Skipped: 1004
[INFO] 

[ERROR] There are test failures.

@viliam-durina
Copy link
Contributor

run-lab-run

@viliam-durina
Copy link
Contributor

I will not backport this to older maintenance branches since the feature was Beta, if that is ok ?

Sure it is.

@sancar sancar merged commit 9545a67 into hazelcast:master Jun 17, 2021
@sancar sancar deleted the fix/portableNewSql/master branch June 17, 2021 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants