Skip to content

Feature parquet extract module - #126

Open
ConorSwainDI wants to merge 22 commits into
mainfrom
parquetExtractModule
Open

Feature parquet extract module#126
ConorSwainDI wants to merge 22 commits into
mainfrom
parquetExtractModule

Conversation

@ConorSwainDI

Copy link
Copy Markdown

Converts an in-memory kdb+ table into one or more .parquet files via kx.arrow. Rows are grouped
by instrument and packed into files close to a configurable target size, splitting any single
oversized instrument across multiple files where required. A manifest recording what was written
(file, instruments, row count, time range, on-disk size) is accumulated in the module's private
manifest table

Comment thread di/pqx/pqx.q
Comment thread di/pqx/pqx.q
Comment thread di/pqx/pqx.q
Comment thread di/pqx/pqx.q Outdated
Comment thread di/pqx/pqx.q
Comment thread di/pqx/pqx.q
Comment thread di/pqx/pqx.q Outdated
Comment thread di/pqx/pqx.q Outdated
@DI-Software-Engineering

Copy link
Copy Markdown

DIReview Summary

3 critical | 5 warning(s) | 0 suggestion(s)

⚠️ Spec check skipped — tracker lookup failed (NO_REF_FOUND). Standards axis only.

Comment thread di/pqx/pqx.q Outdated
Comment thread di/pqx/pqx.q Outdated
@DI-Software-Engineering

Copy link
Copy Markdown

DIReview Summary

0 critical | 2 warning(s) | 0 suggestion(s)

⚠️ Spec check skipped — tracker lookup failed (NO_REF_FOUND). Standards axis only.

Comment thread di/pqx/pqx.q
@DI-Software-Engineering

Copy link
Copy Markdown

DIReview Summary

0 critical | 1 warning(s) | 0 suggestion(s)

⚠️ Spec check skipped — tracker lookup failed (NO_REF_FOUND). Standards axis only.

Comment thread di/pqx/pqx.q
if[any not key[o] in key[default];
.z.m.logerr[`pqx;err:"di.pqx: input keys not recognised - ", "," sv string key[o] where not key[o] in key default];
'err
];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The bad-key check reads key[o] (the raw user-supplied overrides), but o may be an empty dict ()!() or even a general dict where key returns a symbol list. That is fine. However, the check any not key[o] in key[default] will throw a type error if o is passed as a non-dict (e.g. a general list), crashing before the user-friendly error message is produced. More importantly, if o is the null/empty dict the any result is 0b and it passes silently — which is correct. The real issue is that key[default] is recomputed on every extract call from the module-level default dict; if default has been extended elsewhere before this check, a key that was not in the original default will not be caught. This is a fragile dependency on the mutable module-level default. Consider capturing key[default] at init time.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Default should not be extended

Comment thread di/pqx/pqx.q Outdated
Comment thread di/pqx/pqx.q
Comment thread di/pqx/pqx.q
@DI-Software-Engineering

Copy link
Copy Markdown

DIReview Summary

0 critical | 4 warning(s) | 0 suggestion(s)

⚠️ Spec check skipped — tracker lookup failed (NO_REF_FOUND). Standards axis only.

Comment thread di/pqx/pqx.q Outdated
files:([] file:system"find \"",(1 _ string path),"\" -name \"*.parquet\"");
files:update split:"/" vs/:file from files;
lv:1+count where "/"=string path;
levels:(),datecol,dictcols;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The find command is built by string-concatenating 1 _ string path directly into a shell command without any escaping. If hdbdir or tname contains shell metacharacters (spaces, quotes, $, backticks, etc.), this will silently produce wrong results or execute unintended shell commands. Use a safe path construction instead, or at minimum document that path components must not contain shell metacharacters.

Comment thread di/pqx/pqx.q
Comment thread di/pqx/pqx.q
/ casts the xth hive-style path segment (a "key=value" string) of every row in y's split column to its
/ reconstructed value - a date if it's the datecol level, else a symbol
:$[datecol = first `$distinct first each "=" vs' v:y[;x];
"D"$last each "=" vs' v;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

castvirtualcol inspects only the first row's key name (first $distinct first each "=" vs' v) to decide whether the level is the datecol. If the directory happens to contain no files (empty filestable),vis empty andfirstreturns null, causing the comparisondatecol = first null` to produce a type error or wrong result. More critically, it assumes all files at that path depth share the same key name — if they don't (mixed directory layouts), only the first file's key name is consulted, silently miscasting the rest.

@DI-Software-Engineering

Copy link
Copy Markdown

DIReview Summary

0 critical | 3 warning(s) | 0 suggestion(s)

⚠️ Spec check skipped — tracker lookup failed (NO_REF_FOUND). Standards axis only.

Comment thread di/pqx/pqx.q Outdated
@DI-Software-Engineering

Copy link
Copy Markdown

DIReview Summary

1 critical | 0 warning(s) | 0 suggestion(s)

⚠️ Spec check skipped — tracker lookup failed (NO_REF_FOUND). Standards axis only.

Comment thread di/pqx/pqx.q
@DI-Software-Engineering

Copy link
Copy Markdown

DIReview Summary

1 critical | 1 warning(s) | 0 suggestion(s)

⚠️ Spec check skipped — tracker lookup failed (NO_REF_FOUND). Standards axis only.

Comment thread di/pqx/pqx.q
Comment thread di/pqx/test.csv
@DI-Software-Engineering

Copy link
Copy Markdown

DIReview Summary

0 critical | 1 warning(s) | 0 suggestion(s)

⚠️ Spec check skipped — tracker lookup failed (NO_REF_FOUND). Standards axis only.

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