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

Added FORMAT(binary) copy capability and example #204

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cpiker
Copy link

@cpiker cpiker commented Jan 5, 2025

Hi Denizzzka

The dpq2 library met most of my needs, but a small update was added to assist with fast binary copy-in. The changes are:

  1. The function Connection.putCopyData was overloaded to take const(ubyte[]) values since these are common for raw data.
  2. The example.d file was updated to include an explicit binary copy-in example for variable length BYTEA types in a tight loop
  3. example.d was given a dub --single recipe to make it easier to build.

I certainly don't expect you to accept these changes upstream, but wanted to offer them in case they are useful.

Thanks for creating dpq2!

cpiker added 2 commits January 5, 2025 15:24
The function Connection.putCopyData was overloaded to take
const(ubyte[]) values since these are common for raw data.
In addition the example.d file was updated to include an
explicit binary copy in for BYTEA types and to work with dub.
// For FORMAT binary, send over the 19 byte PostgreSQL header manually
// P G C O P Y \n 255 \r \n
conn.putCopyData(cast(ubyte[])[
0x50,0x47,0x43,0x4F,0x50,0x59,0x0A,0xFF,0x0D,0x0A,0,0,0,0,0,0,0,0,0
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe it makes sense to hide this predefined constant in a separate method?

Copy link
Author

@cpiker cpiker Jan 6, 2025

Choose a reason for hiding this comment

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

Oh it certainly does.

This code is an extract of essential features from my application code that uses dpq2. I just wanted to get a working example to you and others for polishing. Preferably by someone who understands the guiding principles of dpq2 better than I.

size_t offset = 0;
buf[].write!(short, BE)(2, &offset); // Sending two fields

buf[].write!(int, BE)(long.sizeof, &offset); // BIGINT == long
Copy link
Owner

@denizzzka denizzzka Jan 6, 2025

Choose a reason for hiding this comment

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

In the dpq2.conv.from_d_types module we have converter for (almost) all supported types. Actually, it converts D values into Value struct, but it is possible to extract needed code into a separate codeblock

@denizzzka
Copy link
Owner

Hello! Thank you very much!

I think this is a good improvement because sometimes it is very necessary to quickly fill a database with a large amount of data. Usually this is a rare operation, but if you need it then there are no workarounds.

@cpiker
Copy link
Author

cpiker commented Jan 6, 2025

Thanks for the encouraging comments :-)

Would it be possible to accept the pull and then convert it right away to a more canonical example? I'm much more confident in your ability than my own in this regard. For example when reading the library I didn't quite understand the BSON features, but I'm guessing they are applicable here?

If this sounds reasonable, then maybe it would make more sense for me to send a pull request to a different branch.

@denizzzka
Copy link
Owner

Would it be possible to accept the pull and then convert it right away to a more canonical example?

I find it hard to write code "for the drawer", because I don't need this functionality now.
You know better what is required. Just start - I will help.

I'm much more confident in your ability than my own in this regard. For example when reading the library I didn't quite understand the BSON features, but I'm guessing they are applicable here?

No, we can decide to not support it at first.

BSON support was added to dpq2 primarely to support displaying of "any type value" - for example, for debugging purposes, etc. Something like Value.as!Bson.toString.

It was later replaced in this capacity by std.variant.Variant (

return this.as!Variant.toString~"::"~oidType.to!string~"("~(format == ValueFormat.TEXT? "t" : "b")~")";
).

It is high time to make Bson support by optional dependency.

@cpiker
Copy link
Author

cpiker commented Jan 6, 2025

I find it hard to write code "for the drawer", because I don't need this functionality now. You know better what is required. Just start - I will help.

Sounds good, will do. Feel free to ignore this pull request. I'll submit another much later this evening.

Also, I'm also seeing issues when building under GDC-13. Namely:

src/dpq2/result.d:749:5: error: alias this for classes/interfaces is deprecated [-Werror=deprecated]
  749 |     alias result this;

I can of course add a compile flag, but would prefer not to if possible. I presume you'd want updates for this issue in a separate pull request? If not I can try to address both topics at once.

It is high time to make Bson support by optional dependency.

Looking over the recursive dependency list for dpq2 I see:

 |-dpq2
    |-derelict-pq
    |  |-libpq   [System Lib]
    |  |    
    |  |-derelict-util
    |
    |-money
    |
    |-vibe-serialization
       |-vibe-container
          |-stdx-allocator
            |-mir-core

as well as Phobos of course. Since I'm carefully tracking dependencies as my project moves into production, how many of these dependencies would no longer be required if I were to use a "no-BSON" version of dpq2?

@denizzzka
Copy link
Owner

denizzzka commented Jan 6, 2025

I can of course add a compile flag, but would prefer not to if possible. I presume you'd want updates for this issue in a separate pull request?

Yes, that would be great!

as well as Phobos of course. Since I'm carefully tracking dependencies as my project moves into production, how many of these dependencies would no longer be required if I were to use a "no-BSON" version of dpq2?

vibe-serialization and its children, I think

Also, money module... It is possible to try to get rid of d-money package too. It will be a similar approach, but smaller

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.

2 participants