Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#163 В запросе отсутствует проверка на NULL для поля, которое может потенциально содержать NULL. #1153

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

disant1
Copy link
Contributor

@disant1 disant1 commented Sep 20, 2022

Что сделано

  • Добавлена проверка наличия проверки на NULL при упорядочивании по полю в запросе.

Чек-лист

Общее:

  • ветка PR обновлена из master и нет конфликтов
  • Тесты-кейсы, JUnit тесты правильного и неправильного состояния
  • Измененные Вами исходники отформатированы в соответствии с конвенцией
  • Авто-аудит (SonarQube и CheckStyle) пройден, покрытие кода хорошее, ошибок нет, плохой код устранен
  • Добавлена запись в ИСТОРИЮ ИЗМЕНЕНИЯ, включаемая в пользовательскую документацию плагина

Если применимо:

  • Пользовательская документация на доп.инструменты написана (на русском)
  • Описание проверок - на двух языках

Закрываемые задачи

Closes #163

@marmyshev marmyshev added the Analyze Включение анализа SonarCloud для PR из форков label Sep 20, 2022
@disant1 disant1 marked this pull request as draft September 20, 2022 09:03
@sonarcloud
Copy link

sonarcloud bot commented Sep 20, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

95.1% 95.1% Coverage
0.0% 0.0% Duplication

@disant1 disant1 marked this pull request as ready for review September 20, 2022 10:19
Comment on lines 22 to +23
#### Запросы
- В запросе отсутствует проверка на NULL для поля, которое может потенциально содержать NULL
Copy link
Collaborator

Choose a reason for hiding this comment

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

нужно пустую строку после подзаголовка

Comment on lines +31 to +34
ORDER BY
QuantityBalance;

SELECT
Copy link
Collaborator

Choose a reason for hiding this comment

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

тут надо разбить на 2 примера - это же не один запрос ведь...

Comment on lines +35 to +38
УПОРЯДОЧИТЬ ПО
КоличествоОстаток;

ВЫБРАТЬ
Copy link
Collaborator

Choose a reason for hiding this comment

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

тут надо разбить на 2 примера - это же не один запрос ведь...

Comment on lines +175 to +183
List<CommonExpression> commonExpressions =
EcoreUtil2.getAllContentsOfType(orderExpression, CommonExpression.class);
orderFields.addAll(commonExpressions);
for (CommonExpression commonExpression : commonExpressions)
{
if (commonExpression instanceof MultiPartCommonExpression)
{
orderFields.remove(((MultiPartCommonExpression)commonExpression).getSourceTable());
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

а тут разве не более правильно сделать? вроде других вариантов нет, но при этом удаляешь 2 лишних цикла.

if (orderExpression.getItem().getExpression() instanceof CommonExpression)
{
  orderFields.add(orderExpression.getItem().getExpression())
 }

@@ -50,6 +50,12 @@ JoinToSubQuery_description = Query join with sub query

JoinToSubQuery_title = Query join with sub query

QueryFieldIsNullCheck_description = The query is missing a NULL check for a field that could potentially contain NULL

QueryFieldIsNullCheck_Query_missing_NULL_check_for_field_potentially_contain_NULL = The query is missing a NULL check for a field that could potentially contain NULL
Copy link
Collaborator

Choose a reason for hiding this comment

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

тут нужно вставить имя поля, иначе в тексте его не найти.

Comment on lines +189 to +193
private List<String> getIsNullMethods()
{
List<String> isNullMethods = Arrays.asList(ISNULL_METHODS.split(METHOD_DELIMITER));
isNullMethods.replaceAll(String::trim);
return isNullMethods;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Это видимо код который хотел делать через параметры но забыл? надо переделать на две константы

Comment on lines +83 to +86
if (monitor.isCanceled())
{
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

монитор нужно проверять в циклах и после выполнения тяжелых действий - см. код ниже

return;
}

List<CommonExpression> orderFields = getOrderFields(sourceTable);
Copy link
Collaborator

Choose a reason for hiding this comment

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

если тут вернули пустую коллекцию - то дальше можно не продолжать.

}

List<CommonExpression> orderFields = getOrderFields(sourceTable);
List<QuerySchemaOperator> operators = sourceTable.getOperators();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Если оператор выборки 1 единственный - дальше можно не продолжать

List<QuerySchemaOperator> operators = sourceTable.getOperators();
for (QuerySchemaOperator operator : operators)
{
List<QuerySchemaExpression> fields = operator.getSelectFields();
Copy link
Collaborator

Choose a reason for hiding this comment

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

наверное нужно проверять только операторы с левым/правым соединением, или полным. Внутренние соединение как раз не может давать null в полях

@marmyshev marmyshev added this to the 0.4 для EDT 2022.2 milestone Oct 21, 2022
@disant1 disant1 marked this pull request as draft November 7, 2022 12:43
@marmyshev marmyshev removed this from the 0.4 для EDT 2022.2 milestone Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analyze Включение анализа SonarCloud для PR из форков
Projects
None yet
2 participants