fix: issue #4935 #4940
Merged
fix: issue #4935 #4940
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughThe changes in this pull request enhance the error handling and field management logic within the logs search functionality in Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- web/src/plugins/logs/Index.vue (2 hunks)
🧰 Additional context used
📓 Learnings (1)
web/src/plugins/logs/Index.vue (1)
Learnt from: bjp232004
PR: openobserve/openobserve#4841
File: web/src/plugins/logs/SearchResult.vue:445-455
Timestamp: 2024-10-21T11:16:45.923Z
Learning: In the OpenObserve project, changes to the `addSearchTerm` function's signature in `web/src/plugins/logs/SearchResult.vue` do not require updating its usage in other components such as `BasicValuesFilter.vue`, `FieldList.vue`, or `SearchBar.vue`.
web/src/plugins/logs/Index.vue
Outdated
| if ( | ||
| (item.expr.type === "column_ref" && | ||
| item.expr?.column?.expr?.value === fieldName) || | ||
| item.expr?.column?.expr?.value === fieldName || item.expr.column === fieldName) || |
Contributor
There was a problem hiding this comment.
Ensure correct logical operator precedence in removeFieldByName.
The condition combines && and || operators without explicit parentheses, which may lead to unintended logic due to operator precedence rules. It's recommended to use parentheses to clearly define the intended logic.
Apply this diff to fix the condition:
- if (
- (item.expr.type === "column_ref" &&
- item.expr?.column?.expr?.value === fieldName || item.expr.column === fieldName) ||
- (item.expr.type === "aggr_func" &&
- item.expr?.args?.expr?.column?.value === fieldName)
- ) {
+ if (
+ (item.expr.type === "column_ref" &&
+ (item.expr?.column?.expr?.value === fieldName || item.expr.column === fieldName)
+ ) ||
+ (item.expr.type === "aggr_func" &&
+ item.expr?.args?.expr?.column?.value === fieldName)
+ ) {📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| item.expr?.column?.expr?.value === fieldName || item.expr.column === fieldName) || | |
| (item.expr?.column?.expr?.value === fieldName || item.expr.column === fieldName)) || |
Contributor
Author
There was a problem hiding this comment.
you are correct and I have updated accordingly
Contributor
There was a problem hiding this comment.
@nikhilsaikethe, glad to hear that you've updated the code accordingly!
((
( ^^)
((")(")
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
web/src/plugins/logs/Index.vue
Outdated
Comment on lines
947
to
949
| new RegExp(`\\b${searchObj.data.stream.selectedStream[0]}\\b`, 'g'), // Wrap only standalone 'default' | ||
| `"${searchObj.data.stream.selectedStream[0]}"` |
Contributor
There was a problem hiding this comment.
Escape stream name in regular expression to handle special characters.
When constructing the regular expression with searchObj.data.stream.selectedStream[0], if the stream name contains special regex characters (e.g., ., *, +), it could lead to incorrect matching or runtime errors. To ensure the regex functions correctly, escape the stream name.
Apply this diff to fix the issue:
+ const streamName = searchObj.data.stream.selectedStream[0].replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
const newQuery = parser
.sqlify(parsedSQL)
.replace(/`/g, "")
.replace(
- new RegExp(`\\b${searchObj.data.stream.selectedStream[0]}\\b`, 'g'), // Wrap only standalone 'default'
+ new RegExp(`\\b${streamName}\\b`, 'g'), // Wrap only standalone stream name
`"${searchObj.data.stream.selectedStream[0]}"`
);Committable suggestion was skipped due to low confidence.
Contributor
Author
There was a problem hiding this comment.
yes used this only
Contributor
There was a problem hiding this comment.
@nikhilsaikethe, great to hear that you've applied the fix!
(_/)
(^.^)
(")_(")
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
nikhilsaikethe
force-pushed
the
issue_4935
branch
from
ba9b203 to
7a7be38
Compare
hengfeiyang
approved these changes
Oct 30, 2024
nikhilsaikethe
force-pushed
the
issue_4935
branch
4 times, most recently
from
f058a65 to
10ef74e
Compare
hengfeiyang
pushed a commit
that referenced
this pull request
Nov 1, 2024
This PR fixes 1.#4935 2.This also tries to add wrap the stream name into double quotes so whenever any field also contain the stream name it ignores that <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced error handling for search results, improving user feedback based on various error states. - Improved accuracy in SQL query modifications related to stream names, preventing unintended replacements. - More precise filtering of fields during removal, enhancing the field management process. - Adjusted histogram interval conditions for better data generation, resulting in finer granularity. - More descriptive error messages for improved user notifications during search operations. - **Bug Fixes** - Refined query cancellation process to notify users effectively if a search query is interrupted. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This was referenced Nov 5, 2024
Merged
This was referenced Nov 13, 2024
This was referenced Dec 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes
1.#4935
2.This also tries to add wrap the stream name into double quotes so whenever any field also contain the stream name it ignores that
Summary by CodeRabbit
New Features
Bug Fixes