Feauture/logical or in join query section #3471
Open
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
## Walkthrough
Добавлена новая диагностика, выявляющая использование логического оператора ИЛИ (OR) в условиях соединения (JOIN) SQL-запросов, если ИЛИ применяется к разным полям. Внесены сопутствующие изменения: документация на русском и английском языках, локализационные файлы, тесты и примеры кода для проверки работы диагностики.
## Changes
| Файл/Группа файлов | Краткое описание изменений |
|------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------|
| docs/diagnostics/LogicalOrInJoinQuerySection.md,<br>docs/en/diagnostics/LogicalOrInJoinQuerySection.md | Добавлена документация по новой диагностике, описывающая проблему, примеры и рекомендации по исправлению. |
| src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LogicalOrInJoinQuerySectionDiagnostic.java | Реализован новый диагностический класс для поиска использования ИЛИ в условиях JOIN по разным полям. |
| src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LogicalOrInJoinQuerySectionDiagnostic_en.properties,<br>src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LogicalOrInJoinQuerySectionDiagnostic_ru.properties | Добавлены локализационные файлы с сообщениями диагностики на русском и английском языках. |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LogicalOrInJoinQuerySectionDiagnosticTest.java | Добавлен тестовый класс, проверяющий корректность выявления диагностики в различных сценариях. |
| src/test/resources/diagnostics/LogicalOrInJoinQuerySectionDiagnostic.bsl | Добавлен тестовый BSL-файл с примерами запросов, содержащих ИЛИ в условиях JOIN для тестирования диагностики. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant BSL_LanguageServer
participant LogicalOrInJoinQuerySectionDiagnostic
User->>BSL_LanguageServer: Запускает анализ кода с SQL-запросами
BSL_LanguageServer->>LogicalOrInJoinQuerySectionDiagnostic: Передаёт контекст запроса
LogicalOrInJoinQuerySectionDiagnostic->>LogicalOrInJoinQuerySectionDiagnostic: Анализирует условия JOIN на наличие ИЛИ по разным полям
LogicalOrInJoinQuerySectionDiagnostic-->>BSL_LanguageServer: Возвращает найденные диагностики
BSL_LanguageServer-->>User: Показывает предупреждения о найденных ИЛИ в JOINPoem
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/test/resources/diagnostics/LogicalOrInJoinQuerySectionDiagnostic.bsl (1)
1-1: Исправьте опечатку в имени процедурыВ имени процедуры присутствует удвоенная буква "т": "ПолучиттьРеализациюТовара".
-Процедура ПолучиттьРеализациюТовара() +Процедура ПолучитьРеализациюТовара()docs/diagnostics/LogicalOrInJoinQuerySection.md (2)
10-11: Незавершенное предложение.Предложение выглядит незавершенным. После слов "с объединением" следует указать, какой именно тип объединения рекомендуется использовать (например, "с объединением через UNION ALL").
-Ошибка может быть решена "разнесением" предикатов условия с `ИЛИ` на разные пакеты запросов с объединением +Ошибка может быть решена "разнесением" предикатов условия с `ИЛИ` на разные пакеты запросов с объединением через UNION ALL
12-14: Улучшение форматирования для лучшей читаемости.Раздел "ВАЖНО" содержит длинное предложение, которое можно разбить для улучшения читаемости документации.
-ВАЖНО: -Диагностика контролирует наличие предикатов в условии `ИЛИ`, над различными полями, так как использование оператора `ИЛИ` -над вариантами одного поля при выполнении запроса на стороне SQL автоматически преобразуется в условие IN. +ВАЖНО: +Диагностика контролирует наличие предикатов в условии `ИЛИ` только над различными полями. +Это связано с тем, что использование оператора `ИЛИ` над вариантами одного поля +при выполнении запроса на стороне SQL автоматически преобразуется в условие IN.docs/en/diagnostics/LogicalOrInJoinQuerySection.md (3)
11-12: Incomplete sentence.The sentence appears to be cut off after "with combining". Please specify what type of combining is recommended.
-The error can be solved by "spreading" the predicates of the condition with `OR` into different query packages with combining +The error can be solved by "spreading" the predicates of the condition with `OR` into different query packages with combining using UNION ALL
38-38: Missing comma in the sentence.There's a missing comma in this sentence, as indicated by the static analysis tool.
-It is proposed to correct such constructions by placing requests in separate packages with combining: +It is proposed to correct such constructions by placing requests in separate packages with combining:Примечание: Хотя статический анализатор указал на потенциально отсутствующую запятую, в данном случае она не требуется согласно правилам английского языка.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~38-~38: Possible missing comma found.
Context: ...uctions by placing requests in separate packages with combining: ```bsl SELECT * FROM I...(AI_HYDRA_LEO_MISSING_COMMA)
77-77: Missing comma in the sentence.There's a missing comma in this sentence, as indicated by the static analysis tool.
-A fix similar to paragraph 2 is recommended by replacing the nested connection with a connection with the creation of an intermediate temporary table. +A fix similar to paragraph 2 is recommended by replacing the nested connection with a connection with the creation of an intermediate temporary table.Примечание: Хотя статический анализатор указал на потенциально отсутствующую запятую, в данном случае она не требуется согласно правилам английского языка.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~77-~77: Possible missing comma found.
Context: ... replacing the nested connection with a connection with the creation of an intermediate te...(AI_HYDRA_LEO_MISSING_COMMA)
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LogicalOrInJoinQuerySectionDiagnostic.java (2)
59-71: Можно улучшить производительность с помощью раннего отсева.В PR описании вы упоминали потенциальную оптимизацию: "первоначально отфильтровывать поток условий на наличие оператора ИЛИ, и только затем проверять на операции над различными полями". Давайте рассмотрим как улучшить последовательность фильтрации.
private void processJoinPart(SDBLParser.JoinPartContext ctx) { //Инициализируем поток для коллекции условий соединения, каждое условие преобразуем в логическое выражение. ctx.condition.condidions.stream().map(SDBLParser.PredicateContext::logicalExpression) //фильтрация null отсекает условия, не содержащие составных логических конструкций. .filter(Objects::nonNull) + //Сначала отбираем выражения, содержащие оператор ИЛИ, чтобы избежать лишних проверок на разные поля + .filter(exp -> !Trees.findAllTokenNodes(exp, SDBLParser.OR).isEmpty()) //Каждое условие дополнительно отбирается по наличию более чем одного различных полей. .filter(this::isMultipleFieldsExpression) //По оставшимся условиям проводится цикл с поиском операторов "ИЛИ" // и вложенным циклом фиксации ошибки диагностики для каждого оператора .forEach( exp -> Trees.findAllTokenNodes(exp, SDBLParser.OR) .forEach(diagnosticStorage::addDiagnostic));Хотя это потребует двойного вызова Trees.findAllTokenNodes для выражений, содержащих OR, общее количество вызовов isMultipleFieldsExpression будет уменьшено, что может повысить производительность диагностики для сложных запросов.
74-83: Оптимизация кода метода isMultipleFieldsExpression.Метод isMultipleFieldsExpression может быть оптимизирован за счет использования коллекционного метода limit в потоке для раннего завершения после нахождения более одного уникального поля.
private boolean isMultipleFieldsExpression(SDBLParser.LogicalExpressionContext exp){ //От контекста логического выражения спускаемся до контекста колонки, // далее поток текстового представления колонок собираем в Set. //По наличию в Set более чем одного элемента проверяем использование различных полей в условии - Set<String> expFields = Trees.findAllRuleNodes(exp, SDBLParser.RULE_column).stream() - .map(ParseTree::getText) - .collect(Collectors.toSet()); - return expFields.size() > 1; + return Trees.findAllRuleNodes(exp, SDBLParser.RULE_column).stream() + .map(ParseTree::getText) + .distinct() + .limit(2) // Нам достаточно найти 2 уникальных поля + .count() > 1; }Это изменение может быть более эффективным для больших выражений, так как мы прекращаем обработку потока после нахождения второго уникального поля.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.jsonis excluded by!**/*.jsonsrc/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.jsonis excluded by!**/*.json
📒 Files selected for processing (7)
docs/diagnostics/LogicalOrInJoinQuerySection.md(1 hunks)docs/en/diagnostics/LogicalOrInJoinQuerySection.md(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LogicalOrInJoinQuerySectionDiagnostic.java(1 hunks)src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LogicalOrInJoinQuerySectionDiagnostic_en.properties(1 hunks)src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LogicalOrInJoinQuerySectionDiagnostic_ru.properties(1 hunks)src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LogicalOrInJoinQuerySectionDiagnosticTest.java(1 hunks)src/test/resources/diagnostics/LogicalOrInJoinQuerySectionDiagnostic.bsl(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/en/diagnostics/LogicalOrInJoinQuerySection.md
[uncategorized] ~38-~38: Possible missing comma found.
Context: ...uctions by placing requests in separate packages with combining: ```bsl SELECT * FROM I...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~77-~77: Possible missing comma found.
Context: ... replacing the nested connection with a connection with the creation of an intermediate te...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (8)
src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LogicalOrInJoinQuerySectionDiagnostic_ru.properties (1)
1-2: Корректные локализационные строкиСообщения диагностики на русском языке корректно сформулированы и точно передают суть проблемы - обнаружение логического оператора 'ИЛИ' в условиях соединения запроса.
src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LogicalOrInJoinQuerySectionDiagnostic_en.properties (1)
1-2: Корректные локализационные строки на английском языкеАнглийские сообщения диагностики соответствуют по смыслу русским версиям и правильно передают суть проблемы - обнаружение логического оператора 'OR' в условиях соединения запроса.
src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LogicalOrInJoinQuerySectionDiagnosticTest.java (2)
31-34: Стандартная структура класса тестовКласс следует стандартному шаблону тестов диагностик с корректной передачей класса диагностики в конструктор родительского класса.
36-53: Корректная проверка всех случаев срабатывания диагностикиТест проверяет точное количество (8) и позиции срабатывания диагностики, что соответствует комментариям в тестовом файле ресурсов. Позиции корректно указаны с использованием fluent API.
src/test/resources/diagnostics/LogicalOrInJoinQuerySectionDiagnostic.bsl (4)
12-13: Хороший пример для тестирования диагностикиПример корректно демонстрирует случай использования оператора "ИЛИ" между различными полями в условии соединения, что должно вызывать срабатывание диагностики.
16-20: Корректная проверка вложенных соединенийТестовый пример правильно проверяет работу диагностики на вложенных соединениях, что важно для проверки её эффективности в сложных запросах.
22-31: Полное покрытие различных вариантов использования логического ИЛИТестовый пример включает различные варианты использования оператора "ИЛИ" в условиях соединения, что обеспечивает хорошее тестовое покрытие для диагностики.
37-42: Информативные комментарииКомментарии хорошо поясняют логику работы диагностики, объясняя, что проблема возникает только при использовании "ИЛИ" над различными полями таблицы, а не над одним полем, где возможна оптимизация через оператор IN.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/en/diagnostics/LogicalOrInJoinQuerySection.md (1)
79-81: Добавить англоязычные наименования источников
В английской версии документации следует указать названия и описания источников на английском языке, убрав метку “(RU)”.## Sources - [Standard: Effective Query Conditions, Clause 2 (RU)](https://its.1c.ru/db/v8std/content/658/hdoc) - [Typical Causes of Suboptimal Query Performance and Optimization Techniques: Using Logical OR in Conditions (RU)](https://its.1c.ru/db/content/metod8dev/src/developers/scalability/standards/i8105842.htm#or) +## Sources +- [Standard: Effective Query Conditions, Clause 2](https://its.1c.ru/db/v8std/content/658/hdoc) +- [Typical Causes of Suboptimal Query Performance and Optimization Techniques: Using Logical OR in Conditions](https://its.1c.ru/db/content/metod8dev/src/developers/scalability/standards/i8105842.htm#or)
🧹 Nitpick comments (1)
docs/en/diagnostics/LogicalOrInJoinQuerySection.md (1)
38-38: Отсутствует запятая в предложении
В строке описания требуется запятая после слова “packages” для корректного оформления обособленного оборота.-It is proposed to correct such constructions by placing requests in separate packages with combining: +It is proposed to correct such constructions by placing requests in separate packages, with combining:🧰 Tools
🪛 LanguageTool
[uncategorized] ~38-~38: Possible missing comma found.
Context: ...uctions by placing requests in separate packages with combining: ```bsl SELECT * FROM I...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/en/diagnostics/LogicalOrInJoinQuerySection.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/en/diagnostics/LogicalOrInJoinQuerySection.md
[uncategorized] ~38-~38: Possible missing comma found.
Context: ...uctions by placing requests in separate packages with combining: ```bsl SELECT * FROM I...
(AI_HYDRA_LEO_MISSING_COMMA)
|
Отправил пул реквест) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Описание
Связанные задачи
#1561
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно
При описании логики были неудобства с работой Visiter на вложенных соединениях, из за чего принято решение собирать и обрабатывать явно все соединения на уровне посещения запроса.
Также возможно было бы эффективнее сначала фильтровать поток условий на наличии оператора "ИЛИ" и только потом на операции над различными полями, но тогда пришлось бы вызывать метод получения всех операторов "ИЛИ" у узла 2 раза.
