Skip to content

Switch ad-hoc queries to arrow_ipc format#4240

Open
Karakatiza666 wants to merge 1 commit intomainfrom
webconsole-arrow
Open

Switch ad-hoc queries to arrow_ipc format#4240
Karakatiza666 wants to merge 1 commit intomainfrom
webconsole-arrow

Conversation

@Karakatiza666
Copy link
Copy Markdown
Contributor

@Karakatiza666 Karakatiza666 commented Jun 27, 2025

I will squash commits before merging.

Part of #4219: Deprecate the JSON format for ad-hoc queries

@Karakatiza666 Karakatiza666 requested a review from gz June 27, 2025 08:49
@Karakatiza666 Karakatiza666 added Web Console Related to the browser based UI javascript Pull requests that update Javascript code adhoc Issue related to ad hoc query processing labels Jun 27, 2025
@Karakatiza666 Karakatiza666 force-pushed the webconsole-arrow branch 2 times, most recently from 56b7606 to 9c0a916 Compare June 27, 2025 09:04
@gz
Copy link
Copy Markdown
Contributor

gz commented Jun 27, 2025

It doesn't look like it answers queries (I tried fraud-detection), revision 9c0a916
https://github.com/user-attachments/assets/50f2fbc1-e59f-4638-ab97-31acd2746cad

@Karakatiza666
Copy link
Copy Markdown
Contributor Author

I think you encountered this one: #4239

Copy link
Copy Markdown
Contributor

@gz gz left a comment

Choose a reason for hiding this comment

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

lgtm make sure to do
some testing before merging (different pipelines with different types (variant map etc.) and queries)

@gz
Copy link
Copy Markdown
Contributor

gz commented Jul 2, 2025

@Karakatiza666 can we merge this?

@Karakatiza666
Copy link
Copy Markdown
Contributor Author

Haven't had the chance to do a few tests with complex types, hopefully tomorrow

Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>
@Karakatiza666
Copy link
Copy Markdown
Contributor Author

Blocked by #4287
PR functions correctly, but triggers the above bug

Copy link
Copy Markdown

@mythical-fred mythical-fred left a comment

Choose a reason for hiding this comment

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

Web-console behavioral changes without tests. Blocked by #4287 per author — but tests are also needed before this lands.

import BigNumber from 'bignumber.js'
import Dayjs from 'dayjs'

const arrowIpcValueToJS = <T extends DataType<Type, any>>(arrowType: Field<T>, value: any) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

New Arrow IPC value conversion logic covers a lot of type cases (int64, timestamps, structs, maps, lists). This is exactly the kind of logic that deserves unit tests — each type case can go wrong independently. Per Gerd's directive (2026-03-04): behavioral changes require tests. Setup: npm install -D vitest @testing-library/svelte jsdom. The pure conversion functions here are ideal for unit testing without any DOM or component setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Issue related to ad hoc query processing javascript Pull requests that update Javascript code Web Console Related to the browser based UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants