-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Return correct dataschema for empty results #13831
base: master
Are you sure you want to change the base?
Conversation
377ea73
to
514c218
Compare
#13057 (all segments pruned on broker) is similar to the problem solved by this PR. We should be able to construct the empty response based on the schema. Do you want to also help fix that? |
@Jackie-Jiang Sure, I can address it. We have two options to solve it:
I'm thinking of going with option (1). Thoughts? Additionally a followup could be to add a broker-side LIMIT_0_PRUNER that will help short-circuit faster instead of routing queries to all servers. |
1 is not always possible in the following cases:
I prefer 2 because we should be able to derive the data type for transform based on input types in order to be SQL compatible. If we cannot get that right now, we can leave a TODO and put a type as placeholder. |
+1 to jackie's idea of leaving the column types as TODOs. We've noticed internally that the bigger issue is we're strictly missing the result table. We don't even use the column types from Pinot since the code usually 1) assumes the types ahead of time or 2) infers it from the json result. But the column names and rows are typically assumed to be there. |
I'm working on the broker side changes as well. I'll create that as a separate PR. |
When all segments are pruned on the server, we construct a empty responseBlock . As this response block is constructed without access to segment data, we default to STRING as the datatype for the column - here is the code link . This problem does not exist for pure Aggregation queries.
This PR fixes that by just processing one segment.
Added tests to verify the code changes.