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

Fast serialization of sql records #812

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mihaibudiu
Copy link

This commit introduces a more efficient way to move data from/to Java when using "SqlRecords" - these are structs with possibly nullable base types as fields. This is implemented by serializing the objects manually into byte[] and transferring the byte[] in a single JNI call, then deserializing it into a DDlogRecord. The reverse path is also supported.

This is a subset of what the flatbufs offers, but the flatbufs do not generate APIs to manipulate records, but rather collections.

Currently the SqlRecord lives in the src project, while the serialization support is in DDlogAPI. This is arguably wrong, we should choose one of the two locations. But this PR is here mostly to solicit an initial review.

We also need to support more base types and probably Tuples as well (needed for primary-key based operation).

The normal way to use the SqlRecord is shown in the test createREcordFromRelation:

        Translator t = this.createInputTables(false);
        DDlogProgram ddprogram = t.getDDlogProgram();
        DDlogRelationDeclaration decl = ddprogram.getRelation("Rt2");
        Assert.assertNotNull(decl);
        DDlogType type = decl.getType();
        DDlogTStruct ts = t.resolveType(type).to(DDlogTStruct.class);
        SqlRecord rec = ts.createEmptyRecord();
        rec.add(2L);

@mihaibudiu
Copy link
Author

@lalithsuresh : please take a look at this API. Note that using the DDlogAPI helpers requires the ddlog dynamic library to be already loaded, and this can happen only once. So the order of the tests is important.

@lalithsuresh
Copy link
Contributor

Thanks @mbudiu-vmw. I'll give this a spin.

@ryzhyk
Copy link
Contributor

ryzhyk commented Nov 17, 2020

@lalithsuresh, @mbudiu-vmw , shall we close this PR for now?

@mihaibudiu
Copy link
Author

I expect this is still useful

@ryzhyk
Copy link
Contributor

ryzhyk commented Nov 17, 2020

If you're ok maintaining this code, we can merge it.

@mihaibudiu
Copy link
Author

Let's first wait for Lalith's comments. Then, if he indeed is using it, it has to be generalized to handle more types.
We may also consider moving it to another C file rather than having it in ddlogapi, but it is simpler to have only one of these.

@mihaibudiu
Copy link
Author

If @lalithsuresh is not using this we can perhaps keep it as a draft PR for a while until we figure out whether it will be ever used.

@lalithsuresh
Copy link
Contributor

lalithsuresh commented Nov 17, 2020

I think we should keep it around as a draft PR for now. We'll get more data when I try out #817 in DCM (larger tables, each with more columns etc).

@mihaibudiu mihaibudiu marked this pull request as draft November 17, 2020 19:14
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.

3 participants