Validate Field and Fields Parameters in Relevance Search Functions - #1067

Merged
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields
Dec 20, 2022
Merged

Validate Field and Fields Parameters in Relevance Search Functions#1067
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields

Conversation

@forestmvey

Copy link
Copy Markdown
Collaborator

Description

Relevance functions that query fields should act similar to how a SELECT query works. If a field is queried that does not exist, a SemanticCheckException should be thrown.

Example Queryies

SELECT * FROM stackexchange_beer WHERE query_string([invalid], 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

SELECT * FROM stackexchange_beer WHERE match(invalid, 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

Issues Resolved

Issue: 613

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@forestmvey
forestmvey requested a review from a team as a code ownerNovember 10, 2022 23:05
@codecov-commenter

codecov-commenter commented Nov 10, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1067 (8fa4d22) into 2.x (2af7321) will decrease coverage by 2.49%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## 2.x #1067 +/- ##
============================================
- Coverage 98.30% 95.81% -2.50% - Complexity 3518 3530 +12 
============================================
Files 342 352 +10 Lines 8694 9376 +682 Branches 554 679 +125 ============================================
+ Hits 8547 8984 +437 - Misses 142 334 +192 - Partials 5 58 +53 
FlagCoverage Δ
query-workbench62.76% <ø> (?)
sql-engine98.31% <100.00%> (+<0.01%)⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted FilesCoverage Δ
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java100.00% <100.00%> (ø)
...h/sql/expression/function/OpenSearchFunctions.java100.00% <100.00%> (ø)
workbench/public/components/app.tsx0.00% <0.00%> (ø)
workbench/public/components/SQLPage/SQLPage.tsx100.00% <0.00%> (ø)
...h/public/components/QueryLanguageSwitch/Switch.tsx85.71% <0.00%> (ø)
workbench/public/components/PPLPage/PPLPage.tsx56.52% <0.00%> (ø)
...ch/public/components/QueryResults/QueryResults.tsx61.60% <0.00%> (ø)
...ublic/components/QueryResults/QueryResultsBody.tsx68.32% <0.00%> (ø)
workbench/public/components/Header/Header.tsx100.00% <0.00%> (ø)
workbench/public/utils/PanelWrapper.tsx100.00% <0.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

MaxKsyunz
MaxKsyunz previously approved these changes Nov 11, 2022
@dai-chendai-chen added the bug Something isn't working label Nov 11, 2022
Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
acarbonetto
acarbonetto previously approved these changes Nov 14, 2022
@forestmvey
forestmvey dismissed stale reviews from acarbonetto and MaxKsyunz via db89bbdNovember 17, 2022 22:38
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 9f5304c to a79dfcdCompareNovember 21, 2022 15:59
@acarbonetto

Copy link
Copy Markdown
Collaborator

@forestmvey can you make sure that we aren't reducing coverage with this PR?

Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 7e08c83 to 8fa4d22CompareDecember 8, 2022 20:13
Signed-off-by: forestmvey <forestv@bitquilltech.com>
@forestmvey
forestmvey changed the base branch from 2.x to mainDecember 16, 2022 00:10
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch from 8fa4d22 to f9aa20eCompareDecember 16, 2022 00:10

@dai-chendai-chen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for the changes!

@MaxKsyunz
MaxKsyunz merged commit d03c176 into opensearch-project:mainDec 20, 2022
opensearch-trigger-botBot pushed a commit that referenced this pull request Dec 20, 2022
…1067)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
@MaxKsyunz
MaxKsyunz deleted the integ-fix-permissive-relevance-fields branch December 20, 2022 19:35
penghuo pushed a commit that referenced this pull request Jan 3, 2023
…1067) (#1199)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
Co-authored-by: Forest Vey <36905077+forestmvey@users.noreply.github.com>
@forestmvey
forestmvey restored the integ-fix-permissive-relevance-fields branch April 11, 2023 15:33
@forestmvey
forestmvey deleted the integ-fix-permissive-relevance-fields branch April 11, 2023 15:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.xbugSomething isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@forestmvey@codecov-commenter@acarbonetto@dai-chen@MaxKsyunz
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Validate Field and Fields Parameters in Relevance Search Functions - #1067

Merged
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields
Dec 20, 2022
Merged

Validate Field and Fields Parameters in Relevance Search Functions#1067
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields

Conversation

@forestmvey

Copy link
Copy Markdown
Collaborator

Description

Relevance functions that query fields should act similar to how a SELECT query works. If a field is queried that does not exist, a SemanticCheckException should be thrown.

Example Queryies

SELECT * FROM stackexchange_beer WHERE query_string([invalid], 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

SELECT * FROM stackexchange_beer WHERE match(invalid, 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

Issues Resolved

Issue: 613

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@forestmvey
forestmvey requested a review from a team as a code ownerNovember 10, 2022 23:05
@codecov-commenter

codecov-commenter commented Nov 10, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1067 (8fa4d22) into 2.x (2af7321) will decrease coverage by 2.49%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## 2.x #1067 +/- ##
============================================
- Coverage 98.30% 95.81% -2.50% - Complexity 3518 3530 +12 
============================================
Files 342 352 +10 Lines 8694 9376 +682 Branches 554 679 +125 ============================================
+ Hits 8547 8984 +437 - Misses 142 334 +192 - Partials 5 58 +53 
FlagCoverage Δ
query-workbench62.76% <ø> (?)
sql-engine98.31% <100.00%> (+<0.01%)⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted FilesCoverage Δ
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java100.00% <100.00%> (ø)
...h/sql/expression/function/OpenSearchFunctions.java100.00% <100.00%> (ø)
workbench/public/components/app.tsx0.00% <0.00%> (ø)
workbench/public/components/SQLPage/SQLPage.tsx100.00% <0.00%> (ø)
...h/public/components/QueryLanguageSwitch/Switch.tsx85.71% <0.00%> (ø)
workbench/public/components/PPLPage/PPLPage.tsx56.52% <0.00%> (ø)
...ch/public/components/QueryResults/QueryResults.tsx61.60% <0.00%> (ø)
...ublic/components/QueryResults/QueryResultsBody.tsx68.32% <0.00%> (ø)
workbench/public/components/Header/Header.tsx100.00% <0.00%> (ø)
workbench/public/utils/PanelWrapper.tsx100.00% <0.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

MaxKsyunz
MaxKsyunz previously approved these changes Nov 11, 2022
@dai-chendai-chen added the bug Something isn't working label Nov 11, 2022
Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
acarbonetto
acarbonetto previously approved these changes Nov 14, 2022
@forestmvey
forestmvey dismissed stale reviews from acarbonetto and MaxKsyunz via db89bbdNovember 17, 2022 22:38
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 9f5304c to a79dfcdCompareNovember 21, 2022 15:59
@acarbonetto

Copy link
Copy Markdown
Collaborator

@forestmvey can you make sure that we aren't reducing coverage with this PR?

Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 7e08c83 to 8fa4d22CompareDecember 8, 2022 20:13
Signed-off-by: forestmvey <forestv@bitquilltech.com>
@forestmvey
forestmvey changed the base branch from 2.x to mainDecember 16, 2022 00:10
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch from 8fa4d22 to f9aa20eCompareDecember 16, 2022 00:10

@dai-chendai-chen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for the changes!

@MaxKsyunz
MaxKsyunz merged commit d03c176 into opensearch-project:mainDec 20, 2022
opensearch-trigger-botBot pushed a commit that referenced this pull request Dec 20, 2022
…1067)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
@MaxKsyunz
MaxKsyunz deleted the integ-fix-permissive-relevance-fields branch December 20, 2022 19:35
penghuo pushed a commit that referenced this pull request Jan 3, 2023
…1067) (#1199)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
Co-authored-by: Forest Vey <36905077+forestmvey@users.noreply.github.com>
@forestmvey
forestmvey restored the integ-fix-permissive-relevance-fields branch April 11, 2023 15:33
@forestmvey
forestmvey deleted the integ-fix-permissive-relevance-fields branch April 11, 2023 15:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.xbugSomething isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@forestmvey@codecov-commenter@acarbonetto@dai-chen@MaxKsyunz
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Validate Field and Fields Parameters in Relevance Search Functions - #1067

Merged
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields
Dec 20, 2022
Merged

Validate Field and Fields Parameters in Relevance Search Functions#1067
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields

Conversation

@forestmvey

Copy link
Copy Markdown
Collaborator

Description

Relevance functions that query fields should act similar to how a SELECT query works. If a field is queried that does not exist, a SemanticCheckException should be thrown.

Example Queryies

SELECT * FROM stackexchange_beer WHERE query_string([invalid], 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

SELECT * FROM stackexchange_beer WHERE match(invalid, 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

Issues Resolved

Issue: 613

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@forestmvey
forestmvey requested a review from a team as a code ownerNovember 10, 2022 23:05
@codecov-commenter

codecov-commenter commented Nov 10, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1067 (8fa4d22) into 2.x (2af7321) will decrease coverage by 2.49%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## 2.x #1067 +/- ##
============================================
- Coverage 98.30% 95.81% -2.50% - Complexity 3518 3530 +12 
============================================
Files 342 352 +10 Lines 8694 9376 +682 Branches 554 679 +125 ============================================
+ Hits 8547 8984 +437 - Misses 142 334 +192 - Partials 5 58 +53 
FlagCoverage Δ
query-workbench62.76% <ø> (?)
sql-engine98.31% <100.00%> (+<0.01%)⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted FilesCoverage Δ
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java100.00% <100.00%> (ø)
...h/sql/expression/function/OpenSearchFunctions.java100.00% <100.00%> (ø)
workbench/public/components/app.tsx0.00% <0.00%> (ø)
workbench/public/components/SQLPage/SQLPage.tsx100.00% <0.00%> (ø)
...h/public/components/QueryLanguageSwitch/Switch.tsx85.71% <0.00%> (ø)
workbench/public/components/PPLPage/PPLPage.tsx56.52% <0.00%> (ø)
...ch/public/components/QueryResults/QueryResults.tsx61.60% <0.00%> (ø)
...ublic/components/QueryResults/QueryResultsBody.tsx68.32% <0.00%> (ø)
workbench/public/components/Header/Header.tsx100.00% <0.00%> (ø)
workbench/public/utils/PanelWrapper.tsx100.00% <0.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

MaxKsyunz
MaxKsyunz previously approved these changes Nov 11, 2022
@dai-chendai-chen added the bug Something isn't working label Nov 11, 2022
Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
acarbonetto
acarbonetto previously approved these changes Nov 14, 2022
@forestmvey
forestmvey dismissed stale reviews from acarbonetto and MaxKsyunz via db89bbdNovember 17, 2022 22:38
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 9f5304c to a79dfcdCompareNovember 21, 2022 15:59
@acarbonetto

Copy link
Copy Markdown
Collaborator

@forestmvey can you make sure that we aren't reducing coverage with this PR?

Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 7e08c83 to 8fa4d22CompareDecember 8, 2022 20:13
Signed-off-by: forestmvey <forestv@bitquilltech.com>
@forestmvey
forestmvey changed the base branch from 2.x to mainDecember 16, 2022 00:10
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch from 8fa4d22 to f9aa20eCompareDecember 16, 2022 00:10

@dai-chendai-chen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for the changes!

@MaxKsyunz
MaxKsyunz merged commit d03c176 into opensearch-project:mainDec 20, 2022
opensearch-trigger-botBot pushed a commit that referenced this pull request Dec 20, 2022
…1067)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
@MaxKsyunz
MaxKsyunz deleted the integ-fix-permissive-relevance-fields branch December 20, 2022 19:35
penghuo pushed a commit that referenced this pull request Jan 3, 2023
…1067) (#1199)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
Co-authored-by: Forest Vey <36905077+forestmvey@users.noreply.github.com>
@forestmvey
forestmvey restored the integ-fix-permissive-relevance-fields branch April 11, 2023 15:33
@forestmvey
forestmvey deleted the integ-fix-permissive-relevance-fields branch April 11, 2023 15:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.xbugSomething isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@forestmvey@codecov-commenter@acarbonetto@dai-chen@MaxKsyunz
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Validate Field and Fields Parameters in Relevance Search Functions - #1067

Merged
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields
Dec 20, 2022
Merged

Validate Field and Fields Parameters in Relevance Search Functions#1067
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields

Conversation

@forestmvey

Copy link
Copy Markdown
Collaborator

Description

Relevance functions that query fields should act similar to how a SELECT query works. If a field is queried that does not exist, a SemanticCheckException should be thrown.

Example Queryies

SELECT * FROM stackexchange_beer WHERE query_string([invalid], 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

SELECT * FROM stackexchange_beer WHERE match(invalid, 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

Issues Resolved

Issue: 613

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@forestmvey
forestmvey requested a review from a team as a code ownerNovember 10, 2022 23:05
@codecov-commenter

codecov-commenter commented Nov 10, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1067 (8fa4d22) into 2.x (2af7321) will decrease coverage by 2.49%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## 2.x #1067 +/- ##
============================================
- Coverage 98.30% 95.81% -2.50% - Complexity 3518 3530 +12 
============================================
Files 342 352 +10 Lines 8694 9376 +682 Branches 554 679 +125 ============================================
+ Hits 8547 8984 +437 - Misses 142 334 +192 - Partials 5 58 +53 
FlagCoverage Δ
query-workbench62.76% <ø> (?)
sql-engine98.31% <100.00%> (+<0.01%)⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted FilesCoverage Δ
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java100.00% <100.00%> (ø)
...h/sql/expression/function/OpenSearchFunctions.java100.00% <100.00%> (ø)
workbench/public/components/app.tsx0.00% <0.00%> (ø)
workbench/public/components/SQLPage/SQLPage.tsx100.00% <0.00%> (ø)
...h/public/components/QueryLanguageSwitch/Switch.tsx85.71% <0.00%> (ø)
workbench/public/components/PPLPage/PPLPage.tsx56.52% <0.00%> (ø)
...ch/public/components/QueryResults/QueryResults.tsx61.60% <0.00%> (ø)
...ublic/components/QueryResults/QueryResultsBody.tsx68.32% <0.00%> (ø)
workbench/public/components/Header/Header.tsx100.00% <0.00%> (ø)
workbench/public/utils/PanelWrapper.tsx100.00% <0.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

MaxKsyunz
MaxKsyunz previously approved these changes Nov 11, 2022
@dai-chendai-chen added the bug Something isn't working label Nov 11, 2022
Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
acarbonetto
acarbonetto previously approved these changes Nov 14, 2022
@forestmvey
forestmvey dismissed stale reviews from acarbonetto and MaxKsyunz via db89bbdNovember 17, 2022 22:38
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 9f5304c to a79dfcdCompareNovember 21, 2022 15:59
@acarbonetto

Copy link
Copy Markdown
Collaborator

@forestmvey can you make sure that we aren't reducing coverage with this PR?

Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 7e08c83 to 8fa4d22CompareDecember 8, 2022 20:13
Signed-off-by: forestmvey <forestv@bitquilltech.com>
@forestmvey
forestmvey changed the base branch from 2.x to mainDecember 16, 2022 00:10
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch from 8fa4d22 to f9aa20eCompareDecember 16, 2022 00:10

@dai-chendai-chen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for the changes!

@MaxKsyunz
MaxKsyunz merged commit d03c176 into opensearch-project:mainDec 20, 2022
opensearch-trigger-botBot pushed a commit that referenced this pull request Dec 20, 2022
…1067)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
@MaxKsyunz
MaxKsyunz deleted the integ-fix-permissive-relevance-fields branch December 20, 2022 19:35
penghuo pushed a commit that referenced this pull request Jan 3, 2023
…1067) (#1199)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
Co-authored-by: Forest Vey <36905077+forestmvey@users.noreply.github.com>
@forestmvey
forestmvey restored the integ-fix-permissive-relevance-fields branch April 11, 2023 15:33
@forestmvey
forestmvey deleted the integ-fix-permissive-relevance-fields branch April 11, 2023 15:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.xbugSomething isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@forestmvey@codecov-commenter@acarbonetto@dai-chen@MaxKsyunz
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Validate Field and Fields Parameters in Relevance Search Functions - #1067

Merged
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields
Dec 20, 2022
Merged

Validate Field and Fields Parameters in Relevance Search Functions#1067
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields

Conversation

@forestmvey

Copy link
Copy Markdown
Collaborator

Description

Relevance functions that query fields should act similar to how a SELECT query works. If a field is queried that does not exist, a SemanticCheckException should be thrown.

Example Queryies

SELECT * FROM stackexchange_beer WHERE query_string([invalid], 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

SELECT * FROM stackexchange_beer WHERE match(invalid, 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

Issues Resolved

Issue: 613

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@forestmvey
forestmvey requested a review from a team as a code ownerNovember 10, 2022 23:05
@codecov-commenter

codecov-commenter commented Nov 10, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1067 (8fa4d22) into 2.x (2af7321) will decrease coverage by 2.49%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## 2.x #1067 +/- ##
============================================
- Coverage 98.30% 95.81% -2.50% - Complexity 3518 3530 +12 
============================================
Files 342 352 +10 Lines 8694 9376 +682 Branches 554 679 +125 ============================================
+ Hits 8547 8984 +437 - Misses 142 334 +192 - Partials 5 58 +53 
FlagCoverage Δ
query-workbench62.76% <ø> (?)
sql-engine98.31% <100.00%> (+<0.01%)⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted FilesCoverage Δ
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java100.00% <100.00%> (ø)
...h/sql/expression/function/OpenSearchFunctions.java100.00% <100.00%> (ø)
workbench/public/components/app.tsx0.00% <0.00%> (ø)
workbench/public/components/SQLPage/SQLPage.tsx100.00% <0.00%> (ø)
...h/public/components/QueryLanguageSwitch/Switch.tsx85.71% <0.00%> (ø)
workbench/public/components/PPLPage/PPLPage.tsx56.52% <0.00%> (ø)
...ch/public/components/QueryResults/QueryResults.tsx61.60% <0.00%> (ø)
...ublic/components/QueryResults/QueryResultsBody.tsx68.32% <0.00%> (ø)
workbench/public/components/Header/Header.tsx100.00% <0.00%> (ø)
workbench/public/utils/PanelWrapper.tsx100.00% <0.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

MaxKsyunz
MaxKsyunz previously approved these changes Nov 11, 2022
@dai-chendai-chen added the bug Something isn't working label Nov 11, 2022
Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
acarbonetto
acarbonetto previously approved these changes Nov 14, 2022
@forestmvey
forestmvey dismissed stale reviews from acarbonetto and MaxKsyunz via db89bbdNovember 17, 2022 22:38
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 9f5304c to a79dfcdCompareNovember 21, 2022 15:59
@acarbonetto

Copy link
Copy Markdown
Collaborator

@forestmvey can you make sure that we aren't reducing coverage with this PR?

Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 7e08c83 to 8fa4d22CompareDecember 8, 2022 20:13
Signed-off-by: forestmvey <forestv@bitquilltech.com>
@forestmvey
forestmvey changed the base branch from 2.x to mainDecember 16, 2022 00:10
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch from 8fa4d22 to f9aa20eCompareDecember 16, 2022 00:10

@dai-chendai-chen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for the changes!

@MaxKsyunz
MaxKsyunz merged commit d03c176 into opensearch-project:mainDec 20, 2022
opensearch-trigger-botBot pushed a commit that referenced this pull request Dec 20, 2022
…1067)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
@MaxKsyunz
MaxKsyunz deleted the integ-fix-permissive-relevance-fields branch December 20, 2022 19:35
penghuo pushed a commit that referenced this pull request Jan 3, 2023
…1067) (#1199)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
Co-authored-by: Forest Vey <36905077+forestmvey@users.noreply.github.com>
@forestmvey
forestmvey restored the integ-fix-permissive-relevance-fields branch April 11, 2023 15:33
@forestmvey
forestmvey deleted the integ-fix-permissive-relevance-fields branch April 11, 2023 15:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.xbugSomething isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@forestmvey@codecov-commenter@acarbonetto@dai-chen@MaxKsyunz
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Validate Field and Fields Parameters in Relevance Search Functions - #1067

Merged
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields
Dec 20, 2022
Merged

Validate Field and Fields Parameters in Relevance Search Functions#1067
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields

Conversation

@forestmvey

Copy link
Copy Markdown
Collaborator

Description

Relevance functions that query fields should act similar to how a SELECT query works. If a field is queried that does not exist, a SemanticCheckException should be thrown.

Example Queryies

SELECT * FROM stackexchange_beer WHERE query_string([invalid], 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

SELECT * FROM stackexchange_beer WHERE match(invalid, 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

Issues Resolved

Issue: 613

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@forestmvey
forestmvey requested a review from a team as a code ownerNovember 10, 2022 23:05
@codecov-commenter

codecov-commenter commented Nov 10, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1067 (8fa4d22) into 2.x (2af7321) will decrease coverage by 2.49%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## 2.x #1067 +/- ##
============================================
- Coverage 98.30% 95.81% -2.50% - Complexity 3518 3530 +12 
============================================
Files 342 352 +10 Lines 8694 9376 +682 Branches 554 679 +125 ============================================
+ Hits 8547 8984 +437 - Misses 142 334 +192 - Partials 5 58 +53 
FlagCoverage Δ
query-workbench62.76% <ø> (?)
sql-engine98.31% <100.00%> (+<0.01%)⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted FilesCoverage Δ
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java100.00% <100.00%> (ø)
...h/sql/expression/function/OpenSearchFunctions.java100.00% <100.00%> (ø)
workbench/public/components/app.tsx0.00% <0.00%> (ø)
workbench/public/components/SQLPage/SQLPage.tsx100.00% <0.00%> (ø)
...h/public/components/QueryLanguageSwitch/Switch.tsx85.71% <0.00%> (ø)
workbench/public/components/PPLPage/PPLPage.tsx56.52% <0.00%> (ø)
...ch/public/components/QueryResults/QueryResults.tsx61.60% <0.00%> (ø)
...ublic/components/QueryResults/QueryResultsBody.tsx68.32% <0.00%> (ø)
workbench/public/components/Header/Header.tsx100.00% <0.00%> (ø)
workbench/public/utils/PanelWrapper.tsx100.00% <0.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

MaxKsyunz
MaxKsyunz previously approved these changes Nov 11, 2022
@dai-chendai-chen added the bug Something isn't working label Nov 11, 2022
Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
acarbonetto
acarbonetto previously approved these changes Nov 14, 2022
@forestmvey
forestmvey dismissed stale reviews from acarbonetto and MaxKsyunz via db89bbdNovember 17, 2022 22:38
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 9f5304c to a79dfcdCompareNovember 21, 2022 15:59
@acarbonetto

Copy link
Copy Markdown
Collaborator

@forestmvey can you make sure that we aren't reducing coverage with this PR?

Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 7e08c83 to 8fa4d22CompareDecember 8, 2022 20:13
Signed-off-by: forestmvey <forestv@bitquilltech.com>
@forestmvey
forestmvey changed the base branch from 2.x to mainDecember 16, 2022 00:10
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch from 8fa4d22 to f9aa20eCompareDecember 16, 2022 00:10

@dai-chendai-chen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for the changes!

@MaxKsyunz
MaxKsyunz merged commit d03c176 into opensearch-project:mainDec 20, 2022
opensearch-trigger-botBot pushed a commit that referenced this pull request Dec 20, 2022
…1067)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
@MaxKsyunz
MaxKsyunz deleted the integ-fix-permissive-relevance-fields branch December 20, 2022 19:35
penghuo pushed a commit that referenced this pull request Jan 3, 2023
…1067) (#1199)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
Co-authored-by: Forest Vey <36905077+forestmvey@users.noreply.github.com>
@forestmvey
forestmvey restored the integ-fix-permissive-relevance-fields branch April 11, 2023 15:33
@forestmvey
forestmvey deleted the integ-fix-permissive-relevance-fields branch April 11, 2023 15:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.xbugSomething isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@forestmvey@codecov-commenter@acarbonetto@dai-chen@MaxKsyunz
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Validate Field and Fields Parameters in Relevance Search Functions - #1067

Merged
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields
Dec 20, 2022
Merged

Validate Field and Fields Parameters in Relevance Search Functions#1067
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields

Conversation

@forestmvey

Copy link
Copy Markdown
Collaborator

Description

Relevance functions that query fields should act similar to how a SELECT query works. If a field is queried that does not exist, a SemanticCheckException should be thrown.

Example Queryies

SELECT * FROM stackexchange_beer WHERE query_string([invalid], 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

SELECT * FROM stackexchange_beer WHERE match(invalid, 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

Issues Resolved

Issue: 613

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@forestmvey
forestmvey requested a review from a team as a code ownerNovember 10, 2022 23:05
@codecov-commenter

codecov-commenter commented Nov 10, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1067 (8fa4d22) into 2.x (2af7321) will decrease coverage by 2.49%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## 2.x #1067 +/- ##
============================================
- Coverage 98.30% 95.81% -2.50% - Complexity 3518 3530 +12 
============================================
Files 342 352 +10 Lines 8694 9376 +682 Branches 554 679 +125 ============================================
+ Hits 8547 8984 +437 - Misses 142 334 +192 - Partials 5 58 +53 
FlagCoverage Δ
query-workbench62.76% <ø> (?)
sql-engine98.31% <100.00%> (+<0.01%)⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted FilesCoverage Δ
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java100.00% <100.00%> (ø)
...h/sql/expression/function/OpenSearchFunctions.java100.00% <100.00%> (ø)
workbench/public/components/app.tsx0.00% <0.00%> (ø)
workbench/public/components/SQLPage/SQLPage.tsx100.00% <0.00%> (ø)
...h/public/components/QueryLanguageSwitch/Switch.tsx85.71% <0.00%> (ø)
workbench/public/components/PPLPage/PPLPage.tsx56.52% <0.00%> (ø)
...ch/public/components/QueryResults/QueryResults.tsx61.60% <0.00%> (ø)
...ublic/components/QueryResults/QueryResultsBody.tsx68.32% <0.00%> (ø)
workbench/public/components/Header/Header.tsx100.00% <0.00%> (ø)
workbench/public/utils/PanelWrapper.tsx100.00% <0.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

MaxKsyunz
MaxKsyunz previously approved these changes Nov 11, 2022
@dai-chendai-chen added the bug Something isn't working label Nov 11, 2022
Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
acarbonetto
acarbonetto previously approved these changes Nov 14, 2022
@forestmvey
forestmvey dismissed stale reviews from acarbonetto and MaxKsyunz via db89bbdNovember 17, 2022 22:38
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 9f5304c to a79dfcdCompareNovember 21, 2022 15:59
@acarbonetto

Copy link
Copy Markdown
Collaborator

@forestmvey can you make sure that we aren't reducing coverage with this PR?

Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 7e08c83 to 8fa4d22CompareDecember 8, 2022 20:13
Signed-off-by: forestmvey <forestv@bitquilltech.com>
@forestmvey
forestmvey changed the base branch from 2.x to mainDecember 16, 2022 00:10
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch from 8fa4d22 to f9aa20eCompareDecember 16, 2022 00:10

@dai-chendai-chen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for the changes!

@MaxKsyunz
MaxKsyunz merged commit d03c176 into opensearch-project:mainDec 20, 2022
opensearch-trigger-botBot pushed a commit that referenced this pull request Dec 20, 2022
…1067)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
@MaxKsyunz
MaxKsyunz deleted the integ-fix-permissive-relevance-fields branch December 20, 2022 19:35
penghuo pushed a commit that referenced this pull request Jan 3, 2023
…1067) (#1199)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
Co-authored-by: Forest Vey <36905077+forestmvey@users.noreply.github.com>
@forestmvey
forestmvey restored the integ-fix-permissive-relevance-fields branch April 11, 2023 15:33
@forestmvey
forestmvey deleted the integ-fix-permissive-relevance-fields branch April 11, 2023 15:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.xbugSomething isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@forestmvey@codecov-commenter@acarbonetto@dai-chen@MaxKsyunz
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Validate Field and Fields Parameters in Relevance Search Functions - #1067

Merged
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields
Dec 20, 2022
Merged

Validate Field and Fields Parameters in Relevance Search Functions#1067
MaxKsyunz merged 1 commit into
opensearch-project:mainfrom
Bit-Quill:integ-fix-permissive-relevance-fields

Conversation

@forestmvey

Copy link
Copy Markdown
Collaborator

Description

Relevance functions that query fields should act similar to how a SELECT query works. If a field is queried that does not exist, a SemanticCheckException should be thrown.

Example Queryies

SELECT * FROM stackexchange_beer WHERE query_string([invalid], 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

SELECT * FROM stackexchange_beer WHERE match(invalid, 'beer');

{'reason': 'Invalid SQL query', 'details': "can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env", 'type': 'SemanticCheckException'}

Issues Resolved

Issue: 613

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@forestmvey
forestmvey requested a review from a team as a code ownerNovember 10, 2022 23:05
@codecov-commenter

codecov-commenter commented Nov 10, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1067 (8fa4d22) into 2.x (2af7321) will decrease coverage by 2.49%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## 2.x #1067 +/- ##
============================================
- Coverage 98.30% 95.81% -2.50% - Complexity 3518 3530 +12 
============================================
Files 342 352 +10 Lines 8694 9376 +682 Branches 554 679 +125 ============================================
+ Hits 8547 8984 +437 - Misses 142 334 +192 - Partials 5 58 +53 
FlagCoverage Δ
query-workbench62.76% <ø> (?)
sql-engine98.31% <100.00%> (+<0.01%)⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted FilesCoverage Δ
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java100.00% <100.00%> (ø)
...h/sql/expression/function/OpenSearchFunctions.java100.00% <100.00%> (ø)
workbench/public/components/app.tsx0.00% <0.00%> (ø)
workbench/public/components/SQLPage/SQLPage.tsx100.00% <0.00%> (ø)
...h/public/components/QueryLanguageSwitch/Switch.tsx85.71% <0.00%> (ø)
workbench/public/components/PPLPage/PPLPage.tsx56.52% <0.00%> (ø)
...ch/public/components/QueryResults/QueryResults.tsx61.60% <0.00%> (ø)
...ublic/components/QueryResults/QueryResultsBody.tsx68.32% <0.00%> (ø)
workbench/public/components/Header/Header.tsx100.00% <0.00%> (ø)
workbench/public/utils/PanelWrapper.tsx100.00% <0.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

MaxKsyunz
MaxKsyunz previously approved these changes Nov 11, 2022
@dai-chendai-chen added the bug Something isn't working label Nov 11, 2022
Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
acarbonetto
acarbonetto previously approved these changes Nov 14, 2022
@forestmvey
forestmvey dismissed stale reviews from acarbonetto and MaxKsyunz via db89bbdNovember 17, 2022 22:38
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 9f5304c to a79dfcdCompareNovember 21, 2022 15:59
@acarbonetto

Copy link
Copy Markdown
Collaborator

@forestmvey can you make sure that we aren't reducing coverage with this PR?

Comment threadcore/src/main/java/org/opensearch/sql/analysis/Analyzer.java Outdated
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch 3 times, most recently from 7e08c83 to 8fa4d22CompareDecember 8, 2022 20:13
Signed-off-by: forestmvey <forestv@bitquilltech.com>
@forestmvey
forestmvey changed the base branch from 2.x to mainDecember 16, 2022 00:10
@forestmvey
forestmveyforce-pushed the integ-fix-permissive-relevance-fields branch from 8fa4d22 to f9aa20eCompareDecember 16, 2022 00:10

@dai-chendai-chen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for the changes!

@MaxKsyunz
MaxKsyunz merged commit d03c176 into opensearch-project:mainDec 20, 2022
opensearch-trigger-botBot pushed a commit that referenced this pull request Dec 20, 2022
…1067)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
@MaxKsyunz
MaxKsyunz deleted the integ-fix-permissive-relevance-fields branch December 20, 2022 19:35
penghuo pushed a commit that referenced this pull request Jan 3, 2023
…1067) (#1199)
Change relevance functions that query fields to throw a SemanticCheckException when a field is queried that does not exist.
Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit d03c176)
Co-authored-by: Forest Vey <36905077+forestmvey@users.noreply.github.com>
@forestmvey
forestmvey restored the integ-fix-permissive-relevance-fields branch April 11, 2023 15:33
@forestmvey
forestmvey deleted the integ-fix-permissive-relevance-fields branch April 11, 2023 15:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.xbugSomething isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@forestmvey@codecov-commenter@acarbonetto@dai-chen@MaxKsyunz