fix: histogram error sql mode #3608
Conversation
WalkthroughThe recent changes involve enhancements to the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant UI as User Interface
participant LogsComposable as useLogs
participant SQLParser as fnHistogramParsedSQL
User ->> UI: Interact with Logs UI
UI ->> LogsComposable: Call useLogs function
LogsComposable ->> SQLParser: Parse SQL query
SQLParser -->> LogsComposable: Return parsed results
LogsComposable ->> UI: Return processed logs data
UI -->> User: Display logs data
This diagram illustrates the high-level interaction between the user interface, the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (3)
tests/ui-testing/playwright-tests/sanity.spec.js (2)
Line range hint
3-3: Consider using thenode:protocol when importing Node.js built-in modules for clarity and to avoid potential resolution issues in different environments.- import { log } from "console"; + import { log } from "node:console";
Line range hint
10-10: Simplify the computed expressions by removing unnecessary string literals.- const folderName = `Folder ${Date.now()}` + const folderName = 'Folder' + Date.now()Also applies to: 15-15, 20-20, 63-63, 66-66, 70-70, 97-97
web/src/composables/useLogs.ts (1)
Line range hint
128-128: Specify more precise types instead ofany.The use of
anytype is found extensively throughout theuseLogsfunction. Usinganybypasses TypeScript's type checking, which can lead to runtime errors. Where possible, replaceanywith more specific types to enhance code reliability and maintainability.Also applies to: 129-129, 139-139, 143-143, 146-146, 151-151, 153-153, 154-154, 155-155, 156-156, 157-157, 169-169, 170-170, 175-175, 176-176, 186-186, 187-187, 223-223, 235-235, 236-236
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- tests/ui-testing/playwright-tests/sanity.spec.js (2 hunks)
- web/src/composables/useLogs.ts (3 hunks)
Additional Context Used
Biome (30)
tests/ui-testing/playwright-tests/sanity.spec.js (10)
3-3: A Node.js builtin module should be imported with the node: protocol.
10-10: The computed expression can be simplified without the use of a string literal.
15-15: The computed expression can be simplified without the use of a string literal.
20-20: The computed expression can be simplified without the use of a string literal.
37-37: Unexpected control character(s) in regular expression: \x00
63-63: The computed expression can be simplified without the use of a string literal.
66-66: The computed expression can be simplified without the use of a string literal.
66-66: The computed expression can be simplified without the use of a string literal.
70-70: The computed expression can be simplified without the use of a string literal.
97-97: The computed expression can be simplified without the use of a string literal.
web/src/composables/useLogs.ts (20)
128-128: Unexpected any. Specify a different type.
129-129: Unexpected any. Specify a different type.
139-139: Unexpected any. Specify a different type.
143-143: Unexpected any. Specify a different type.
146-146: Unexpected any. Specify a different type.
151-151: Unexpected any. Specify a different type.
153-153: Unexpected any. Specify a different type.
154-154: Unexpected any. Specify a different type.
155-155: Unexpected any. Specify a different type.
156-156: Unexpected any. Specify a different type.
157-157: Unexpected any. Specify a different type.
169-169: Unexpected any. Specify a different type.
170-170: Unexpected any. Specify a different type.
175-175: Unexpected any. Specify a different type.
176-176: Unexpected any. Specify a different type.
186-186: Unexpected any. Specify a different type.
187-187: Unexpected any. Specify a different type.
223-223: Unexpected any. Specify a different type.
235-235: Unexpected any. Specify a different type.
236-236: Unexpected any. Specify a different type.
Additional comments not posted (3)
tests/ui-testing/playwright-tests/sanity.spec.js (3)
Line range hint
37-37: The regular expression used to remove UTF characters might include unexpected control characters. Ensure the regex pattern is correctly specified.Verification successful
The regex pattern
[^\x00-\x7F]correctly matches non-ASCII characters, including control characters outside the ASCII range (0x80 to 0x9F). This confirms that the pattern is functioning as intended and does not inadvertently include unexpected control characters.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the regex pattern for removing UTF characters. echo "Testing regex pattern..." echo "Sample text with special characters: ñ, é, ü" | grep -P "[^\x00-\x7F]"Length of output: 182
Script:
#!/bin/bash # Description: Verify the regex pattern for removing UTF characters, including control characters outside the ASCII range. echo "Testing regex pattern with control characters outside ASCII range..." # Create a string with control characters outside the ASCII range (0x80 to 0x9F) control_chars=$(printf '\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8A\x8B\x8C\x8D\x8E\x8F\x90\x91\x92\x93\x94\x95\x96\x97\x98\x99\x9A\x9B\x9C\x9D\x9E\x9F') echo "Control characters: $control_chars" | grep -P "[^\x00-\x7F]"Length of output: 575
195-198: Ensure that the reset and refresh interactions are functioning as intended, especially since these are new additions according to the PR summary.Verification successful
The locators for the reset and refresh buttons are correctly identified and interacted with in the test file. The interactions are present at the specified lines, confirming that the buttons are being clicked as intended.
- Line 195: Reset button interaction
- Line 197: Refresh button interaction
- Additional refresh button interactions at lines 202 and 433
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the reset and refresh button functionality in the UI. echo "Manual verification needed: Check the reset and refresh interactions on the UI."Length of output: 168
Script:
#!/bin/bash # Search for the reset button interaction in the test file ast-grep --lang javascript --pattern $'await page.locator(\'[data-test="logs-search-bar-reset-filters-btn"]\').click()' tests/ui-testing/playwright-tests/sanity.spec.js # Search for the refresh button interaction in the test file ast-grep --lang javascript --pattern $'await page.locator(\'[data-test="logs-search-bar-refresh-btn"]\').click()' tests/ui-testing/playwright-tests/sanity.spec.jsLength of output: 859
425-444: The new interactions to display the histogram in SQL mode seem to be implemented correctly. However, ensure that the histogram displays accurately and that no errors are thrown when toggling SQL mode.
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
web/src/composables/useLogs.ts (1)
Line range hint
128-128: Replaceanytype with more specific types to improve type safety.- let parser: any; + let parser: SQLParserType; // Assuming SQLParserType is the correct type - const fieldValues = ref(); + const fieldValues = ref<FieldValueType>(); // Replace FieldValueType with the actual type // Apply similar changes to all other instances where `any` is used.Also applies to: 129-129, 139-139, 143-143, 146-146, 151-151, 153-153, 154-154, 155-155, 156-156, 157-157, 169-169, 170-170, 175-175, 176-176, 186-186, 187-187, 223-223, 235-235, 236-236
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- web/src/composables/useLogs.ts (9 hunks)
Additional Context Used
Biome (20)
web/src/composables/useLogs.ts (20)
128-128: Unexpected any. Specify a different type.
129-129: Unexpected any. Specify a different type.
139-139: Unexpected any. Specify a different type.
143-143: Unexpected any. Specify a different type.
146-146: Unexpected any. Specify a different type.
151-151: Unexpected any. Specify a different type.
153-153: Unexpected any. Specify a different type.
154-154: Unexpected any. Specify a different type.
155-155: Unexpected any. Specify a different type.
156-156: Unexpected any. Specify a different type.
157-157: Unexpected any. Specify a different type.
169-169: Unexpected any. Specify a different type.
170-170: Unexpected any. Specify a different type.
175-175: Unexpected any. Specify a different type.
176-176: Unexpected any. Specify a different type.
186-186: Unexpected any. Specify a different type.
187-187: Unexpected any. Specify a different type.
223-223: Unexpected any. Specify a different type.
235-235: Unexpected any. Specify a different type.
236-236: Unexpected any. Specify a different type.
Additional comments not posted (3)
web/src/composables/useLogs.ts (3)
632-647: Refactor SQL manipulation to use helper functions.
1407-1423: Introduce error handling infnHistogramParsedSQL.
826-826: Ensure proper handling of non-aggregated queries.
| req.aggs.histogram = req.aggs.histogram.replace( | ||
| "[INDEX_NAME]", | ||
| searchObj.data.stream.selectedStream.value | ||
| ); | ||
|
|
||
| req.aggs.histogram = req.aggs.histogram.replace("[WHERE_CLAUSE]", ""); | ||
|
|
||
| searchObj.data.query = query; | ||
| const parsedSQL: any = fnParsedSQL(); | ||
| const histogramParsedSQL: any = fnHistogramParsedSQL(req.aggs.histogram); | ||
|
|
||
| histogramParsedSQL.where = parsedSQL.where; | ||
|
|
||
| let histogramQuery = parser.sqlify(histogramParsedSQL) | ||
| histogramQuery = histogramQuery.replace(/`/g, '"'); | ||
| req.aggs.histogram = histogramQuery; |
There was a problem hiding this comment.
Refactor complex SQL string manipulations in buildSearch.
- req.aggs.histogram = req.aggs.histogram.replace("[INDEX_NAME]", searchObj.data.stream.selectedStream.value);
- req.aggs.histogram = req.aggs.histogram.replace("[WHERE_CLAUSE]", "");
+ req.aggs.histogram = replaceIndexName(req.aggs.histogram, searchObj.data.stream.selectedStream.value);
+ req.aggs.histogram = replaceWhereClause(req.aggs.histogram, "");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| req.aggs.histogram = req.aggs.histogram.replace( | |
| "[INDEX_NAME]", | |
| searchObj.data.stream.selectedStream.value | |
| ); | |
| req.aggs.histogram = req.aggs.histogram.replace("[WHERE_CLAUSE]", ""); | |
| searchObj.data.query = query; | |
| const parsedSQL: any = fnParsedSQL(); | |
| const histogramParsedSQL: any = fnHistogramParsedSQL(req.aggs.histogram); | |
| histogramParsedSQL.where = parsedSQL.where; | |
| let histogramQuery = parser.sqlify(histogramParsedSQL) | |
| histogramQuery = histogramQuery.replace(/`/g, '"'); | |
| req.aggs.histogram = histogramQuery; | |
| req.aggs.histogram = replaceIndexName(req.aggs.histogram, searchObj.data.stream.selectedStream.value); | |
| req.aggs.histogram = replaceWhereClause(req.aggs.histogram, ""); | |
| searchObj.data.query = query; | |
| const parsedSQL: any = fnParsedSQL(); | |
| const histogramParsedSQL: any = fnHistogramParsedSQL(req.aggs.histogram); | |
| histogramParsedSQL.where = parsedSQL.where; | |
| let histogramQuery = parser.sqlify(histogramParsedSQL) | |
| histogramQuery = histogramQuery.replace(/`/g, '"'); | |
| req.aggs.histogram = histogramQuery; |
bjp232004
force-pushed
the
sql-mode-histogram-error
branch
from
f4ec88f to
a1be890
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
tests/ui-testing/playwright-tests/sanity.spec.js (1)
Line range hint
3-3: Consider using thenode:protocol when importing Node.js built-in modules for clarity and to avoid potential issues with module resolution.- import { log } from "console"; + import { log } from "node:console";web/src/composables/useLogs.ts (1)
Line range hint
128-128: Consider replacinganytype with more specific types to improve type safety and code maintainability.Also applies to: 129-129, 139-139, 143-143, 146-146, 151-151, 153-153, 154-154, 155-155, 156-156, 157-157, 169-169, 170-170, 175-175, 176-176, 186-186, 187-187, 223-223, 235-235, 236-236
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- tests/ui-testing/playwright-tests/sanity.spec.js (2 hunks)
- web/src/composables/useLogs.ts (10 hunks)
- web/src/plugins/logs/Index.vue (1 hunks)
Additional Context Used
Biome (30)
tests/ui-testing/playwright-tests/sanity.spec.js (10)
3-3: A Node.js builtin module should be imported with the node: protocol.
10-10: The computed expression can be simplified without the use of a string literal.
15-15: The computed expression can be simplified without the use of a string literal.
20-20: The computed expression can be simplified without the use of a string literal.
37-37: Unexpected control character(s) in regular expression: \x00
63-63: The computed expression can be simplified without the use of a string literal.
66-66: The computed expression can be simplified without the use of a string literal.
66-66: The computed expression can be simplified without the use of a string literal.
70-70: The computed expression can be simplified without the use of a string literal.
97-97: The computed expression can be simplified without the use of a string literal.
web/src/composables/useLogs.ts (20)
128-128: Unexpected any. Specify a different type.
129-129: Unexpected any. Specify a different type.
139-139: Unexpected any. Specify a different type.
143-143: Unexpected any. Specify a different type.
146-146: Unexpected any. Specify a different type.
151-151: Unexpected any. Specify a different type.
153-153: Unexpected any. Specify a different type.
154-154: Unexpected any. Specify a different type.
155-155: Unexpected any. Specify a different type.
156-156: Unexpected any. Specify a different type.
157-157: Unexpected any. Specify a different type.
169-169: Unexpected any. Specify a different type.
170-170: Unexpected any. Specify a different type.
175-175: Unexpected any. Specify a different type.
176-176: Unexpected any. Specify a different type.
186-186: Unexpected any. Specify a different type.
187-187: Unexpected any. Specify a different type.
223-223: Unexpected any. Specify a different type.
235-235: Unexpected any. Specify a different type.
236-236: Unexpected any. Specify a different type.
Additional comments not posted (5)
tests/ui-testing/playwright-tests/sanity.spec.js (2)
448-467: Ensure that the histogram displays correctly in SQL mode and handles errors gracefully.
196-199: Ensure proper refresh and reset functionality.web/src/plugins/logs/Index.vue (1)
432-432: Clarify the impact of commenting outloadLogsData()on the application's behavior.web/src/composables/useLogs.ts (2)
632-647: Refactor complex SQL string manipulations inbuildSearch.- req.aggs.histogram = req.aggs.histogram.replace("[INDEX_NAME]", searchObj.data.stream.selectedStream.value); - req.aggs.histogram = req.aggs.histogram.replace("[WHERE_CLAUSE]", ""); + req.aggs.histogram = replaceIndexName(req.aggs.histogram, searchObj.data.stream.selectedStream.value); + req.aggs.histogram = replaceWhereClause(req.aggs.histogram, "");
1407-1423: Optimize SQL parsing infnHistogramParsedSQL.- const filteredQuery = query - .split("\n") - .filter((line: string) => !line.trim().startsWith("--")) - .join("\n"); + const filteredQuery = query.replace(/^--.*$/gm, ''); // Use regex to remove comments more efficiently
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
web/src/composables/useLogs.ts (1)
Line range hint
128-236: Consider replacinganytype with more specific types to improve type safety.- let parser: any; + let parser: SQLParserType; // Assuming SQLParserType is the correct type, replace with actual type if different - const fieldValues = ref(); + const fieldValues = ref<FieldType[]>(); // Replace FieldType with the actual expected type - const initialQueryPayload: Ref<LogsQueryPayload | null> = ref(null); + const initialQueryPayload: Ref<LogsQueryPayload> = ref(); // Specify the type directly in the ref declaration // Apply similar changes to other instances where `any` is used.Consider defining specific types for variables currently typed as
any. This will enhance the code's type safety and maintainability, making it easier to understand and less prone to runtime errors.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- tests/ui-testing/playwright-tests/sanity.spec.js (3 hunks)
- web/src/composables/useLogs.ts (10 hunks)
Additional Context Used
Biome (30)
tests/ui-testing/playwright-tests/sanity.spec.js (10)
3-3: A Node.js builtin module should be imported with the node: protocol.
10-10: The computed expression can be simplified without the use of a string literal.
15-15: The computed expression can be simplified without the use of a string literal.
20-20: The computed expression can be simplified without the use of a string literal.
37-37: Unexpected control character(s) in regular expression: \x00
63-63: The computed expression can be simplified without the use of a string literal.
66-66: The computed expression can be simplified without the use of a string literal.
66-66: The computed expression can be simplified without the use of a string literal.
70-70: The computed expression can be simplified without the use of a string literal.
97-97: The computed expression can be simplified without the use of a string literal.
web/src/composables/useLogs.ts (20)
128-128: Unexpected any. Specify a different type.
129-129: Unexpected any. Specify a different type.
139-139: Unexpected any. Specify a different type.
143-143: Unexpected any. Specify a different type.
146-146: Unexpected any. Specify a different type.
151-151: Unexpected any. Specify a different type.
153-153: Unexpected any. Specify a different type.
154-154: Unexpected any. Specify a different type.
155-155: Unexpected any. Specify a different type.
156-156: Unexpected any. Specify a different type.
157-157: Unexpected any. Specify a different type.
169-169: Unexpected any. Specify a different type.
170-170: Unexpected any. Specify a different type.
175-175: Unexpected any. Specify a different type.
176-176: Unexpected any. Specify a different type.
186-186: Unexpected any. Specify a different type.
187-187: Unexpected any. Specify a different type.
223-223: Unexpected any. Specify a different type.
235-235: Unexpected any. Specify a different type.
236-236: Unexpected any. Specify a different type.
Additional comments not posted (3)
tests/ui-testing/playwright-tests/sanity.spec.js (1)
196-199: Ensure consistent use of UI interactions.Verification successful
The verification process has shown that interactions involving the reset filters button and the refresh button are used consistently across different tests. The specific interactions in the
sanity.spec.jsfile align with similar interactions found in other test files.
- Interactions with the reset filters button were found only in
sanity.spec.js.- Interactions with the refresh button were found in multiple test files, including
sanity.spec.js,functions.spec.js,variable.spec.js,region.spec.js,reports.spec.js,logsquickmode.spec.js,pipeline.spec.js,logsqueries.spec.js,indexquery.spec.js,logspage.spec.js,dashboardtype.spec.js,alerts.spec.js, andschema.spec.js.This consistency indicates that the UI interactions for resetting filters and refreshing the page are uniformly implemented across the test suite.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the interactions for resetting filters and refreshing the page are consistent across different tests. # Test: Search for similar interactions in other tests. Expect: Consistent usage. rg --type js $'page.locator.*click.*refresh'Length of output: 45
Script:
#!/bin/bash # Description: Verify that the interactions for resetting filters and refreshing the page are consistent across different tests. # Test: Search for interactions involving the reset filters button. rg --type js 'page.locator.*reset-filters.*click' # Test: Search for interactions involving the refresh button. rg --type js 'page.locator.*refresh.*click'Length of output: 5153
web/src/composables/useLogs.ts (2)
632-647: Refactor complex SQL string manipulations inbuildSearch.
1407-1423: Optimize SQL parsing infnHistogramParsedSQL.
…rror as end-time and start_time is string not a number
bjp232004
force-pushed
the
sql-mode-histogram-error
branch
from
ba8a83e to
8d66728
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (2)
tests/ui-testing/playwright-tests/sanity.spec.js (2)
Line range hint
3-3: Consider using thenode:protocol when importing Node.js built-in modules.- import { log } from "console"; + import { log } from "node:console";
Line range hint
37-37: Remove unexpected control characters in the regular expression.- return text.replace(/[^\x00-\x7F]/g, " "); + return text.replace(/[^\x00-\x7F]/g, ""); // Removed the space in the replacement string
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- tests/ui-testing/playwright-tests/sanity.spec.js (3 hunks)
- web/src/composables/useLogs.ts (11 hunks)
Additional Context Used
Biome (30)
tests/ui-testing/playwright-tests/sanity.spec.js (10)
3-3: A Node.js builtin module should be imported with the node: protocol.
10-10: The computed expression can be simplified without the use of a string literal.
15-15: The computed expression can be simplified without the use of a string literal.
20-20: The computed expression can be simplified without the use of a string literal.
37-37: Unexpected control character(s) in regular expression: \x00
63-63: The computed expression can be simplified without the use of a string literal.
66-66: The computed expression can be simplified without the use of a string literal.
66-66: The computed expression can be simplified without the use of a string literal.
70-70: The computed expression can be simplified without the use of a string literal.
97-97: The computed expression can be simplified without the use of a string literal.
web/src/composables/useLogs.ts (20)
128-128: Unexpected any. Specify a different type.
129-129: Unexpected any. Specify a different type.
139-139: Unexpected any. Specify a different type.
143-143: Unexpected any. Specify a different type.
146-146: Unexpected any. Specify a different type.
151-151: Unexpected any. Specify a different type.
153-153: Unexpected any. Specify a different type.
154-154: Unexpected any. Specify a different type.
155-155: Unexpected any. Specify a different type.
156-156: Unexpected any. Specify a different type.
157-157: Unexpected any. Specify a different type.
169-169: Unexpected any. Specify a different type.
170-170: Unexpected any. Specify a different type.
175-175: Unexpected any. Specify a different type.
176-176: Unexpected any. Specify a different type.
186-186: Unexpected any. Specify a different type.
187-187: Unexpected any. Specify a different type.
223-223: Unexpected any. Specify a different type.
235-235: Unexpected any. Specify a different type.
236-236: Unexpected any. Specify a different type.
Additional comments not posted (3)
tests/ui-testing/playwright-tests/sanity.spec.js (1)
449-468: Ensure error handling is consistent across similar test cases.web/src/composables/useLogs.ts (2)
632-647: Ensure consistent SQL string manipulation.The changes made here to replace the SQL string manipulations with function calls are good for maintainability and readability. This should help prevent errors and make the code easier to manage.
Line range hint
128-128: Specify explicit types instead ofany.
[REFACTOR_SUGGESTion]
Usinganytype defeats the purpose of TypeScript's static type checking. Where possible, replaceanywith more specific types to improve code safety and clarity.- let parser: any; + let parser: SQLParserType; // Assuming SQLParserType is an appropriate type - const fieldValues = ref(); + const fieldValues = ref<Record<string, unknown>>(); // Adjust the generic parameter as neededAlso applies to: 129-129, 139-139, 143-143, 146-146, 151-151, 153-153, 154-154, 155-155, 156-156, 157-157, 169-169, 170-170, 175-175, 176-176, 186-186, 187-187, 223-223, 235-235, 236-236
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (4)
tests/ui-testing/playwright-tests/sanity.spec.js (3)
Line range hint
3-3: Consider using thenode:protocol when importing Node.js built-in modules.- import { log } from "console"; + import { log } from "node:console";
Line range hint
10-10: Simplify computed expressions to enhance readability and performance.- const folderName = `Folder ${Date.now()}` + const folderName = 'Folder' + Date.now();Also applies to: 15-15, 20-20, 63-63, 66-66, 70-70, 97-97
Line range hint
37-37: Ensure that the regular expression does not contain unexpected control characters.Please verify and correct the regular expression used in the
removeUTFCharactersfunction to ensure it behaves as expected.web/src/composables/useLogs.ts (1)
Line range hint
128-128: Replaceanytype with more specific types to improve type safety.- let parser: any; + let parser: SQLParser; - const fieldValues = ref(); + const fieldValues = ref<Record<string, unknown>>(); - const initialQueryPayload: Ref<LogsQueryPayload | null> = ref(null); + const initialQueryPayload: Ref<LogsQueryPayload> = ref<LogsQueryPayload>({}); - const useSqlParser: any = await import("@/composables/useParser"); + const useSqlParser: { sqlParser: () => SQLParser } = await import("@/composables/useParser"); - const { sqlParser }: any = useSqlParser.default(); + const { sqlParser }: { sqlParser: () => SQLParser } = useSqlParser.default(); - const streamData: any = await getStream(streamName, searchObj.data.stream.streamType || "logs", true); + const streamData: StreamData = await getStream(streamName, searchObj.data.stream.streamType || "logs", true); - const parsedSQL: any = fnParsedSQL(); + const parsedSQL: ParsedSQL = fnParsedSQL(); - const itemObj: { + const itemObj: StreamItem = { name: any; args: string; }; - const schemaFields: any = []; + const schemaFields: string[] = []; - let userDefineSchemaSettings: any = []; + let userDefineAlso applies to: 129-129, 139-139, 143-143, 146-146, 151-151, 153-153, 154-154, 155-155, 156-156, 157-157, 169-169, 170-170, 175-175, 176-176, 186-186, 187-187, 223-223, 235-235, 236-236
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- tests/ui-testing/playwright-tests/sanity.spec.js (2 hunks)
- web/src/composables/useLogs.ts (11 hunks)
- web/src/plugins/logs/Index.vue (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- web/src/plugins/logs/Index.vue
Additional Context Used
Biome (30)
tests/ui-testing/playwright-tests/sanity.spec.js (10)
3-3: A Node.js builtin module should be imported with the node: protocol.
10-10: The computed expression can be simplified without the use of a string literal.
15-15: The computed expression can be simplified without the use of a string literal.
20-20: The computed expression can be simplified without the use of a string literal.
37-37: Unexpected control character(s) in regular expression: \x00
63-63: The computed expression can be simplified without the use of a string literal.
66-66: The computed expression can be simplified without the use of a string literal.
66-66: The computed expression can be simplified without the use of a string literal.
70-70: The computed expression can be simplified without the use of a string literal.
97-97: The computed expression can be simplified without the use of a string literal.
web/src/composables/useLogs.ts (20)
128-128: Unexpected any. Specify a different type.
129-129: Unexpected any. Specify a different type.
139-139: Unexpected any. Specify a different type.
143-143: Unexpected any. Specify a different type.
146-146: Unexpected any. Specify a different type.
151-151: Unexpected any. Specify a different type.
153-153: Unexpected any. Specify a different type.
154-154: Unexpected any. Specify a different type.
155-155: Unexpected any. Specify a different type.
156-156: Unexpected any. Specify a different type.
157-157: Unexpected any. Specify a different type.
169-169: Unexpected any. Specify a different type.
170-170: Unexpected any. Specify a different type.
175-175: Unexpected any. Specify a different type.
176-176: Unexpected any. Specify a different type.
186-186: Unexpected any. Specify a different type.
187-187: Unexpected any. Specify a different type.
223-223: Unexpected any. Specify a different type.
235-235: Unexpected any. Specify a different type.
236-236: Unexpected any. Specify a different type.
Additional comments not posted (2)
web/src/composables/useLogs.ts (2)
632-647: Refactor complex SQL string manipulations inbuildSearch.- req.aggs.histogram = req.aggs.histogram.replace("[INDEX_NAME]", searchObj.data.stream.selectedStream.value); - req.aggs.histogram = req.aggs.histogram.replace("[WHERE_CLAUSE]", ""); + req.aggs.histogram = replaceIndexName(req.aggs.histogram, searchObj.data.stream.selectedStream.value); + req.aggs.histogram = replaceWhereClause(req.aggs.histogram, "");
826-826: Consider checking for aggregation in SQL queries to optimize performance.Verification successful
The search results indicate that there are indeed checks for SQL aggregation in the codebase, specifically within the
useLogs.tsanduseQuery.tsfiles. The presence of theisNonAggregatedQueryfunction and SQL aggregation keywords likeGROUP BYandCOUNTsuggests that the code is handling aggregation checks.
web/src/composables/useLogs.ts: Contains theisNonAggregatedQueryfunction and multiple references to it.web/src/composables/useQuery.ts: Contains SQL aggregation keywords.Given this context, it appears that the code is already considering SQL aggregation checks.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if SQL queries are properly handling aggregation checks. # Test: Search for SQL aggregation checks. Expect: Proper handling of aggregation checks. rg --type typescript $'isNonAggregatedQuery'Length of output: 78
Script:
#!/bin/bash # Description: Verify if SQL queries are properly handling aggregation checks. # Test: Search for SQL aggregation checks. Expect: Proper handling of aggregation checks. rg --type ts 'isNonAggregatedQuery|GROUP BY|SUM|COUNT|AVG|MIN|MAX'Length of output: 1187
| test(' should display histogram in sql mode', async ({ page }) => { | ||
| await page.locator('[data-test="logs-search-result-bar-chart"] canvas').click({ | ||
| position: { | ||
| x: 182, | ||
| y: 66 | ||
| } | ||
| }); | ||
| await page.getByLabel('SQL Mode').locator('div').nth(2).click(); | ||
| await page.locator('[data-test="logs-search-bar-refresh-btn"]').click(); | ||
| // await page.getByRole('heading', { name: 'Error while fetching' }).click(); | ||
|
|
||
| await expect(page.getByRole('heading', { name: 'Error while fetching' })).not.toBeVisible(); | ||
| await page.locator('[data-test="logs-search-result-bar-chart"] canvas').click({ | ||
| position: { | ||
| x: 182, | ||
| y: 66 | ||
| } | ||
| }); | ||
|
|
||
| }); |
There was a problem hiding this comment.
Tip
Codebase Verification
Ensure consistent error handling in histogram display logic.
- The
sanity.spec.jsfile includes a check to ensure 'Error while fetching' is not visible. - The
logspage.spec.jsfile lacks similar error handling checks.
Please add error handling in logspage.spec.js to ensure consistency.
Analysis chain
Ensure consistent error handling in histogram display logic.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the histogram display logic in SQL mode is correctly implemented and does not cause errors.
# Test: Search for similar logic in other parts of the codebase. Expect: No errors related to histogram display.
rg --type js $'page.locator.*logs-search-result-bar-chart.*click'
Length of output: 598
Script:
#!/bin/bash
# Extract relevant parts of logspage.spec.js to review error handling in histogram display logic
# Display lines around the found occurrences to understand the context
rg --type js --context 10 'page.locator.*logs-search-result-bar-chart.*click' tests/ui-testing/playwright-tests/logspage.spec.js
Length of output: 1739
| const fnHistogramParsedSQL = (query: string) => { | ||
| try { | ||
| const filteredQuery = query | ||
| .split("\n") | ||
| .filter((line: string) => !line.trim().startsWith("--")) | ||
| .join("\n"); | ||
| return parser.astify(filteredQuery); | ||
| } catch (e: any) { | ||
| return { | ||
| columns: [], | ||
| orderby: null, | ||
| limit: null, | ||
| groupby: null, | ||
| where: null, | ||
| }; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Introduce error handling for SQL parsing in fnHistogramParsedSQL.
- return parser.astify(filteredQuery);
+ try {
+ return parser.astify(filteredQuery);
+ } catch (e) {
+ console.error("Failed to parse SQL in fnHistogramParsedSQL", e);
+ return {
+ columns: [],
+ orderby: null,
+ limit: null,
+ groupby: null,
+ where: null,
+ };
+ }Committable suggestion was skipped due low confidence.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
loadLogsData()function call in the logs module to optimize performance.