Skip to content

replace clickhouse-cpp with clickhouse-c#254

Open
serprex wants to merge 2 commits into
mainfrom
binary-c
Open

replace clickhouse-cpp with clickhouse-c#254
serprex wants to merge 2 commits into
mainfrom
binary-c

Conversation

@serprex
Copy link
Copy Markdown
Member

@serprex serprex commented May 21, 2026

eradicates #241

@serprex serprex requested a review from theory May 21, 2026 22:24
@serprex serprex force-pushed the binary-c branch 11 times, most recently from 17d92e9 to 500c05b Compare May 22, 2026 14:35
Comment thread README.md
Comment thread README.md Outdated
clickhouse-c is an experimental clickhouse library focused on minimalism,
allowing for proper compatibility reusing postgres allocator
Comment thread src/binary.c
return false;
}

/* dynamic buffer for raw bytes; palloc-backed, freed by context delete */
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

could use StringInfoData if we're okay choking on blocks larger than 1GB

Comment thread src/binary.c

struct ch_binary_state
{
MemoryContext cxt;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@theory I know you've pointed out that this sort of thing belongs in a library, but hooking into MemoryContext / ereport / etc will hopefully convince you of the cleanliness keeping this out of clickhouse-c. maybe we need a src/binary/ with binary/binary_decode/binary_encode/binary_connection/select/insert/...

Comment thread src/binary.c
continue;
}

/* Linear search current dict (skip slot 0 for Nullable). */
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this might be trusting user to understand what "low cardinality" means too much

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