-
Notifications
You must be signed in to change notification settings - Fork 619
[Dashboard] Update insight query group_by from time to block_timestamp #8000
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -31,8 +31,8 @@ export async function getContractEventAnalytics(params: { | |||||||||
| endDate?: Date; | ||||||||||
| }): Promise<AnalyticsEntry[]> { | ||||||||||
| const queryParams = [ | ||||||||||
| `chain=${params.chainId}`, | ||||||||||
| "group_by=time", | ||||||||||
| `chain_id=${params.chainId}`, | ||||||||||
| "group_by=block_timestamp", | ||||||||||
| "aggregate=toStartOfDay(toDate(block_timestamp)) as time", | ||||||||||
|
Comment on lines
+35
to
36
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The change from This mismatch between the grouping field and aggregation alias might cause issues if the backend expects consistent naming between these clauses. Consider either:
This would ensure the query parameters maintain logical consistency and reduce the risk of unexpected behavior.
Suggested change
Spotted by Diamond
Comment on lines
+34
to
36
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainDaily buckets will fragment: grouping by raw block_timestamp yields per-block groups. If the intent is daily aggregation, - `chain_id=${params.chainId}`,
- "group_by=block_timestamp",
- "aggregate=toStartOfDay(toDate(block_timestamp)) as time",
+ `chain_id=${params.chainId}`,
+ "group_by=toStartOfDay(toDate(block_timestamp)) as time",Optionally normalize count: - "aggregate=count(block_timestamp) as count",
+ "aggregate=count() as count",🌐 Web query: 💡 Result: Short answer: No — the group_by parameter must reference an existing field/alias (or the API's documented filter_alias), not an arbitrary SQL expression with an inline alias. And time filters depend on the provider: some Insight APIs (e.g. Dune) expect date/time strings (use timestamp/date strings in filters), while others (e.g. AWS X‑Ray Insights) require Unix epoch seconds — check the specific API docs. [1][2] Sources:
Group by existing alias, not inline SQL 🤖 Prompt for AI Agents |
||||||||||
| "aggregate=count(block_timestamp) as count", | ||||||||||
| params.startDate | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same grouping issue: per-day breakdown should group by the day expression, not raw block_timestamp.
Group by the day bucket (aliased
time) alongsideevent_signature, and remove the separateaggregatefortime.Optionally normalize count:
📝 Committable suggestion
🤖 Prompt for AI Agents