Skip to content

fix(query): согласовать календарные функции с локальной зоной - #1409

Open
ivanarama wants to merge 5 commits into
mainfrom
fix/1243
Open

ivanarama wants to merge 5 commits into
mainfrom
fix/1243

Conversation

@ivanarama

Copy link
Copy Markdown
Owner

Что исправлено

  • SQLite переводит UTC-момент поля date в локальные стенные часы приложения до Год/Месяц/День, Начало* и КонецДня.
  • Каждый PostgreSQL-коннект получает TimeZone приложения, поэтому TIMESTAMPTZ считает календарные части в той же зоне, что DSL и UI.
  • Локализация применяется только к доказанному типу date и системному Период; обычная строка и будущий localdate остаются «как записаны».

Вариант: 1 (решение человека в комментарии к заявке).

Проверки

  • новый матричный тест query.Compilequery.Run для SQLite/PostgreSQL, Asia/Kolkata и America/New_York, зимнего и летнего смещений;
  • go test для internal/query, internal/storage, internal/dsl/interpreter;
  • зональные прогоны internal/query и internal/storage с TZ=Asia/Kolkata и TZ=America/New_York;
  • go test -race ./internal/query -run '^TestDateFunctionsUseApplicationLocalTime$' -count=1;
  • go build ./...;
  • go test ./....

Локально TEST_DATABASE_URL не задан, поэтому PostgreSQL-половину нового матричного теста исполнит обязательный postgres-integration job.

Совместимость

Для существующих запросов календарный день date у границы UTC может измениться: теперь он совпадает с уже действующей семантикой DSL/UI. Сохранённые моменты не переписываются.

Fixes #1243

Generated-with: Codex

PP-Fix-Transition: from=cdce3214691a599b3500f9c583ac3d36e8822768 review-comment=5591541549 claim=5591542049 epoch-sha256=9b538f8c0e578dd6626a61fcde6669d9802f52bc0208ee41d579f916c224579c
@ivanarama

Copy link
Copy Markdown
Owner Author

Ревью. (круг 1)
Reviewed-SHA: cdce321
Outcome-Label: changes-requested
Что меняется: SQLite локализует доказанные date-аргументы календарных функций через ob_local_datetime; PostgreSQL-соединения получают session TimeZone приложения; добавлен матричный тест SQLite/PostgreSQL..
Проверено: Проверен полный diff PR #1409 на HEAD cdce321 (6 файлов).; go test -count=1 ./internal/query ./internal/storage ./internal/dsl/interpreter — успешно.; TZ=Asia/Kolkata и TZ=America/New_York: go test -count=1 ./internal/query ./internal/storage — успешно.; go test -race ./internal/query -run '^TestDateFunctionsUseApplicationLocalTime$' -count=1 — успешно.; go build ./... — успешно.; go test -count=1 ./... — успешно; TEST_DATABASE_URL локально не задан.; Все обязательные GitHub checks текущего HEAD, включая postgres-integration и test-windows, завершились успешно..
Блокирующее: 1) internal/storage/pg.go:170-195 теряет правила DST для обычной системной зоны. На Windows Go инициализирует time.Local с именем "Local", даже когда внутри есть переходы standard/daylight; при незаданном TZ applicationTimeZoneName сводит такую зону к единственному текущему offset. Например, соединение в America/New_York, открытое летом, получит -04:00 и для январского момента 2026-01-15 23:30 (-05:00, то есть 04:30Z) вычислит в PostgreSQL уже 2026-01-16 00:30, тогда как SQLite/DSL сохранят 15 января. Новый матричный тест это скрывает, потому что подменяет time.Local результатом time.LoadLocation с IANA-именем. Нужно сохранять зональные правила для системной Local (особенно на основной Windows-платформе) и покрыть путь без TZ/IANA-имени..
Хвост:

Вердикт: есть замечания.

@ivanarama

Copy link
Copy Markdown
Owner Author

PromptPilot service marker: REVIEW result publication claimed.

@ivanarama ivanarama added the changes-requested Ревью: есть замечания — фиксер дорабатывает label Sep 8, 2026
@ivanarama

Copy link
Copy Markdown
Owner Author

PromptPilot service marker: REVIEW result committed.

Generated-with: Codex

PP-Fix-Transition: from=023a84ce4163137ea253ec3aa0f9cd168c6fc54e review-comment=5593169883 claim=5593170485 epoch-sha256=2217d1bd6ceccc562b83a2ad3dcc40f6dc3eb32cad6563855a78f9619108fddc
@ivanarama

Copy link
Copy Markdown
Owner Author

Исправлено по блокирующему замечанию:

  • для системной зоны time.Local с именем Local PostgreSQL теперь получает POSIX-описание обеих зон и правил DST, а не фиксированное текущее смещение;
  • добавлен unit-тест точного контракта для America/New_York;
  • публичный матричный тест CompileRun теперь включает путь Local без TZ и проверяет зимнее и летнее смещения.

Осознанно не менял путь именованных IANA-зон и зон без DST: они сохраняют прежнее поведение. Пунктов «Хвоста» в ревью не было.

Проверки:

  • go test -count=1 ./internal/query ./internal/storage ./internal/dsl/interpreter — успешно;
  • go build ./... — успешно;
  • go test -race -count=1 ./internal/query -run '^TestDateFunctionsUseApplicationLocalTime$' — успешно;
  • go test -count=1 ./... — единственный сбой в известном мигающем Windows-тесте TestWatchGoContext_IgnoresProjectSources; отдельный повтор -count=3 прошёл 3/3.

@ivanarama ivanarama removed the changes-requested Ревью: есть замечания — фиксер дорабатывает label Sep 8, 2026
@ivanarama

Copy link
Copy Markdown
Owner Author

Ревью. (круг 2)
Reviewed-SHA: 023a84c
Outcome-Label: changes-requested
Что меняется: SQLite приводит доказанные date-аргументы календарных функций к локальным стенным часам; PostgreSQL-соединения получают TimeZone приложения. Второй коммит сохраняет правила DST для системной зоны Local через POSIX-описание.
Проверено: полный diff 7 файлов; go test -count=1 ./internal/query ./internal/storage ./internal/dsl/interpreter; те же query/storage при TZ=Asia/Kolkata и TZ=America/New_York; race-тест TestDateFunctionsUseApplicationLocalTime; go build ./...; go test -count=1 ./... — всё успешно. Отдельный публичный Compile-пробник воспроизвёл замечание ниже. CI: все обязательные проверки, кроме lint, зелёные; необязательный bench красный.
Блокирующее:

  1. internal/query/query.go:4414: dateArgumentIsMoment для квалифицированного Другой.Значение смотрит только на карту полей первого источника. Если у первого источника одноимённое поле имеет тип date, а у JOIN-источника это string, День(Другой.Значение) ошибочно компилируется как ob_local_datetime(другой.значение). Диагностический тест через публичный query.Compile получил именно этот SQL. Нужно определять тип по квалификатору источника и добавить регрессионный тест с одноимёнными полями разных типов.
  2. Обязательный CI-check lint красный: date_funcs_local_matrix_test.go:132 и pg_timezone_test.go:27 игнорируют ошибку zones.Close, а оба вызова runtime.GOROOT() помечены SA1019. До зелёного lint PR не может быть влит.
    Хвост:

    Вердикт: есть замечания.

@ivanarama

Copy link
Copy Markdown
Owner Author

PromptPilot service marker: REVIEW result publication claimed.

@ivanarama ivanarama added the changes-requested Ревью: есть замечания — фиксер дорабатывает label Sep 8, 2026
@ivanarama

Copy link
Copy Markdown
Owner Author

PromptPilot service marker: REVIEW result committed.

@ivanarama

Copy link
Copy Markdown
Owner Author

Исправлено по блокирующим замечаниям второго круга:

  1. Календарные функции SQLite теперь определяют тип квалифицированного поля по его источнику. Добавлен регрессионный тест через публичный query.Compile с одноимёнными полями date и string в разных JOIN-источниках.
  2. В обоих тестовых помощниках ошибка zones.Close больше не игнорируется, а GOROOT получается через go env GOROOT, что устраняет SA1019.

Осознанно не менял семантику неквалифицированных полей, параметров и путь именованных IANA-зон: они не относятся к блокерам. Пунктов «Хвоста» в ревью не было.

Проверки:

  • go test -count=1 ./internal/query ./internal/storage ./internal/dsl/interpreter — успешно;
  • go test -race -count=1 ./internal/query -run '^(TestDateFunctionsUseApplicationLocalTime|TestDateFunctionUsesQualifiedSourceFieldType)$' — успешно;
  • go vet ./... и go build ./... — успешно;
  • go test -count=1 ./... — успешно;
  • golangci-lint v2.11.4 для internal/query и internal/storage0 issues.

@ivanarama ivanarama removed the changes-requested Ревью: есть замечания — фиксер дорабатывает label Sep 9, 2026
@ivanarama

Copy link
Copy Markdown
Owner Author

Ревью. (круг 3)
Reviewed-SHA: 197c431
Outcome-Label: needs-decision
Что меняется: SQLite приводит доказанные date-аргументы календарных функций к локальным стенным часам, а PostgreSQL-соединения получают TimeZone приложения. Последний коммит выбирает тип квалифицированного поля по источнику и устраняет замечания lint.
Проверено: полный diff 7 файлов; go test -count=1 ./internal/query ./internal/storage ./internal/dsl/interpreter; те же query/storage при TZ=Asia/Kolkata и TZ=America/New_York; race-тесты TestDateFunctionsUseApplicationLocalTime и TestDateFunctionUsesQualifiedSourceFieldType; go vet ./...; go build ./...; go test -count=1 ./... — успешно. Диагностические тесты через публичный query.Compile для вложенного запроса и ОБЪЕДИНИТЬ воспроизвели блокер ниже.
Блокирующее:

  1. internal/query/query.go:2796-2850: карта типов квалификаторов строится одна на весь поток токенов, хотя одинаковый алиас в подзапросе и внешнем запросе либо в разных ветках ОБЪЕДИНИТЬ относится к разным областям. Если Т.Значение имеет тип date в одной области и string в другой, addQualifier помечает Т неоднозначным глобально; оба корректных запроса компилируются без ob_local_datetime, и календарная функция над доказанным date снова считает UTC. Нужно сделать вывод типов областно-зависимым и закрепить оба случая регрессионными тестами.
    Хвост:

    Вердикт: есть замечания.
    Человеку: это третий круг; нужно решить — довести областно-зависимый вывод типов в этом PR или явно сузить заявленный контракт и принять оставшееся расхождение.

@ivanarama

Copy link
Copy Markdown
Owner Author

PromptPilot service marker: REVIEW result publication claimed.

@ivanarama ivanarama added the needs-decision Нужно решение Ивана label Sep 9, 2026
@ivanarama

Copy link
Copy Markdown
Owner Author

PromptPilot service marker: REVIEW result committed.

@ivanarama

Copy link
Copy Markdown
Owner Author

Решение человека: довести областно-зависимый вывод типов в этом PR. Сужать контракт #1243 и принимать расхождение для вложенных запросов или ОБЪЕДИНИТЬ нельзя: это оставит неверную календарную семантику для доказанного типа date. Исправьте область видимости квалификаторов и добавьте оба регрессионных теста из третьего ревью. После исправления PR возвращается в REVIEW.

@ivanarama ivanarama added changes-requested Ревью: есть замечания — фиксер дорабатывает and removed needs-decision Нужно решение Ивана changes-requested Ревью: есть замечания — фиксер дорабатывает labels Sep 9, 2026
Generated-with: Codex

PP-Fix-Transition: from=197c4316794de4ee67d71a702f867e05e480306d review-comment=5594460337 claim=5594473320 epoch-sha256=06560b688df0c9283a0719935d32244135be8ab50d8b68c59ac70fe5b555ad36
@ivanarama

Copy link
Copy Markdown
Owner Author

Решение человека: довести областно-зависимый вывод типов в этом PR. Сужать контракт #1243 нельзя. Исправьте области видимости квалификаторов для вложенных запросов и веток ОБЪЕДИНИТЬ и добавьте оба регрессионных теста из третьего ревью.

pp:fix-decision 197c431

@ivanarama ivanarama added the changes-requested Ревью: есть замечания — фиксер дорабатывает label Sep 9, 2026
@ivanarama

Copy link
Copy Markdown
Owner Author

Исправлено по решению человека и блокирующему замечанию третьего круга:

  • вывод типов квалифицированных полей теперь изолирован по SELECT-scope;
  • одинаковый алиас во вложенном и внешнем запросах больше не делает тип неоднозначным;
  • одинаковый алиас в ветках ОБЪЕДИНИТЬ определяется независимо;
  • добавлены регрессионные тесты через публичный query.Compile для обоих сценариев.

Осознанно не менял неквалифицированные поля, параметры и семантику строк: они не относятся к замечанию. Пунктов «Хвоста» в ревью не было.

Проверки:

  • go build ./...;
  • go test -count=1 ./internal/query ./internal/storage ./internal/dsl/interpreter;
  • go test -race -count=1 ./internal/query для календарных regression-тестов;
  • go test -count=1 ./...;
  • go vet ./...;
  • git diff --check.

@ivanarama ivanarama removed the changes-requested Ревью: есть замечания — фиксер дорабатывает label Sep 9, 2026
@ivanarama

Copy link
Copy Markdown
Owner Author

Ревью. (круг 4)
Reviewed-SHA: fa153fc
Outcome-Label: needs-decision
Что меняется: SQLite локализует доказанные date-аргументы календарных функций, PostgreSQL-соединения получают TimeZone приложения. Последний коммит изолирует квалифицированные типы по SELECT-scope и добавляет тесты вложенного запроса и ОБЪЕДИНИТЬ.
Проверено: полный diff 7 файлов; go test для internal/query, internal/storage и internal/dsl/interpreter; query/storage при TZ=Asia/Kolkata и TZ=America/New_York; race-тесты календарных регрессий; go vet ./...; go build ./...; go test ./... -count=1; git diff --check — штатные проверки успешны. Дополнительный тест через публичный query.Compile воспроизвёл блокер ниже в двух сценариях; обязательные CI-проверки зелёные.
Блокирующее:

  1. internal/query/query.go:3819 и :4510 продолжают передавать одну глобальную карту buildColTypes во все SELECT-scope для неквалифицированных аргументов. Во вложенном запросе с внешним string-полем Значение и внутренним date-полем Значение, а также в ОБЪЕДИНИТЬ веток date/string компилятор поставил ob_local_datetime дважды вместо одного раза: строковое поле меняет смысл вопреки заявленному типозависимому контракту. Областная карта нового коммита исправляет только Т.Значение; обычное День(Значение) осталось зависимым от первого источника всего потока.
    Хвост:

    Вердикт: есть замечания.
    Человеку: это повтор после решения третьего круга; нужно решить — довести областно-зависимый вывод типа также для неквалифицированных полей в этом PR или явно сузить контракт, оставив описанное расхождение. Рекомендую довести вывод типов: иначе обычная строка локализуется как момент.

@ivanarama

Copy link
Copy Markdown
Owner Author

PromptPilot service marker: REVIEW result publication claimed.

@ivanarama ivanarama added the needs-decision Нужно решение Ивана label Sep 9, 2026
@ivanarama

Copy link
Copy Markdown
Owner Author

PromptPilot service marker: REVIEW result committed.

@ivanarama

Copy link
Copy Markdown
Owner Author

Решение по вопросу круга 4: доводим областно-зависимый вывод типа до неквалифицированных полей в этом PR (вариант «а»).

Контракт доработки:

  1. Тип неквалифицированного аргумента календарной функции (День(Значение), Месяц(Значение), …) определяется в области компилируемого SELECT, а не по глобальной карте имён потока: internal/query/query.go:3819 и :4510 перестают передавать один buildColTypes во все scope.
  2. Поле с одинаковым именем, но разными типами в разных областях (внешний string-Значение, внутренний date-Значение) разрешается по своей области: локализация ob_local_datetime применяется ровно один раз к моменту-типу и не применяется к строке.
  3. Внутри одной области неоднозначность (одно имя — разные типы, например ветки ОБЪЕДИНИТЬ date/string) не угадывается: локализация не применяется, компиляция идёт в прежней семантике; предупреждение компилятора о неоднозначном типе — желательно, но не обязательно в объёме этого PR.
  4. Регрессии обязательны на публичном query.Compile для обоих воспроизведённых сценариев из ревью (вложенный запрос string-снаружи/date-внутри; ОБЪЕДИНИТЬ date/string) — ob_local_datetime встречается ровно один раз.
  5. Зелёный обязательный postgres-integration входит в объём доработки.

После доработки — обычный круг REVIEW и мой ship.

@ivanarama ivanarama removed the needs-decision Нужно решение Ивана label Sep 12, 2026
@ivanarama

Copy link
Copy Markdown
Owner Author

Возврат PR в FIX. Решение по кругу 4 выше принято (вариант «а» — доводим областно-зависимый вывод типа до неквалифицированных полей в этом PR), но метка needs-decision была снята без маркера, и PR выпал из всех очередей.

Контракт доработки — по тому решению:

  1. Тип неквалифицированного аргумента календарной функции определяется в области компилируемого SELECT, а не по глобальной карте имён потока: internal/query/query.go:3819 и :4510 перестают передавать один buildColTypes во все scope.
  2. Поле с одинаковым именем и разными типами в разных областях разрешается по своей области: ob_local_datetime применяется ровно один раз к моменту-типу и не применяется к строке.
  3. Внутри одной области неоднозначность не угадывается: локализация не применяется, компиляция идёт в прежней семантике. Предупреждение о неоднозначном типе желательно, но в объём этого PR не входит.
  4. Регрессии на публичном query.Compile для обоих воспроизведённых сценариев ревью — вложенный запрос string-снаружи/date-внутри и ОБЪЕДИНИТЬ date/string; ob_local_datetime встречается ровно один раз.
  5. Зелёный обязательный postgres-integration входит в объём доработки.

После доработки — обычный круг REVIEW.

pp:fix-decision fa153fc

@ivanarama ivanarama added needs-decision Нужно решение Ивана changes-requested Ревью: есть замечания — фиксер дорабатывает and removed needs-decision Нужно решение Ивана labels Sep 13, 2026
Тип неквалифицированного аргумента календарной функции определяла одна
карта buildColTypes, снятая по первому источнику всего потока токенов.
Поэтому во вложенном запросе и в соседних ветках ОБЪЕДИНИТЬ одноимённое
поле чужой области решало за своё: строковое Значение получало
ob_local_datetime, а localizация появлялась дважды вместо одного раза.

Теперь неквалифицированные имена типизирует buildScopedColTypes —
карта на каждую область SELECT, собранная из источников самой области.
Имя, которое два источника одной области типизируют по-разному, из карты
выпадает: компиляция остаётся в прежней семантике, а не угадывает тип
первого источника. Квалифицированные имена и tr.colTypes не тронуты.

Регрессии — на публичном query.Compile: вложенный запрос
string-снаружи/date-внутри, ветки ОБЪЕДИНИТЬ date/string (в обоих
ob_local_datetime ровно один раз) и неоднозначное имя в одной области
(локализации нет).

Generated-with: Claude Code

PP-Fix-Transition: from=fa153fcee99d050c990ec29ab73cc67b97f77492 review-comment=5605530776 claim=5605549587 epoch-sha256=088e194a1f541fa08c8186b528a17bc5cf6600565d592a40b33934657f15b517

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PSKmvmK49phRMJAcyhtBap
@ivanarama

Copy link
Copy Markdown
Owner Author

Доработка по решению круга 4 — вариант «а»: областно-зависимый вывод типа доведён до неквалифицированных полей.

Что сделано:

  1. buildScopedColTypes (internal/query/query.go) — карта типов на каждую область SELECT, собранная из источников самой области. rewriteScalarFuncs получает её вместо единой buildColTypes и типизирует аргумент календарной функции по scopeIDAt того же токена; квалифицированные имена по-прежнему идут через qualifiedColTypes[scopeID].
  2. Одноимённое поле разных областей разрешается своей областью: во вложенном запросе внешнее string-Значение не локализуется, внутреннее date-Значение локализуется — ob_local_datetime ровно один раз. То же для соседних веток ОБЪЕДИНИТЬ.
  3. Неоднозначность внутри одной области не угадывается: имя, которое два источника области типизируют по-разному, выпадает из карты, и локализация не применяется — прежняя семантика, fail-closed.
  4. Регрессии на публичном query.Compile (internal/query/date_funcs_local_matrix_test.go): вложенный запрос string-снаружи/date-внутри, ОБЪЕДИНИТЬ date/string и неоднозначное имя в одной области. До правки первые два сценария давали ob_local_datetime дважды — проверено откатом query.go на исходный HEAD.

Осознанно не менял:

  • tr.colTypes остаётся глобальной. Она отвечает за квалификацию собственных колонок и CAST(... AS NUMERIC) (пп. 48–49), к типу аргумента календарной функции отношения не имеет; её областная разбивка — отдельная работа за рамками решения круга 4.
  • Предупреждение компилятора о неоднозначном типе не добавлял: по п. 3 решения оно желательно, но в объём этого PR не входит.
  • PostgreSQL не затронут: локализация применяется только при dialect == "sqlite", поэтому поведение postgres-integration меняться не должно.

Проверки локально: go build ./...; полный go test ./... -count=1 — зелёный (PostgreSQL-подтесты пропускаются без сервера, их закрывает обязательный postgres-integration); golangci-lint run ./internal/query/... — 0 issues; git diff --check чист.

@ivanarama ivanarama removed the changes-requested Ревью: есть замечания — фиксер дорабатывает label Sep 14, 2026
@ivanarama

Copy link
Copy Markdown
Owner Author

Ревью. (круг 5)
Reviewed-SHA: 1958035
Outcome-Label: needs-decision
Что меняется: SQLite локализует доказанные date-аргументы календарных функций, а PostgreSQL-соединения получают TimeZone приложения. Последний коммит переносит типизацию неквалифицированных полей на область SELECT и добавляет регрессии вложенного запроса и ОБЪЕДИНИТЬ.
Проверено: полный diff 7 файлов; go build ./...; go vet ./...; git diff --check; go test -count=1 ./internal/query в default, Asia/Kolkata и America/New_York; storage-DST тест; race-набор календарных регрессий; go test -count=1 ./internal/dsl/interpreter; go run ./cmd/onebase check --project examples/trade — успешно. Полный локальный go test ./... при параллельной I/O-нагрузке упёрся в несвязанные Windows FlushFileBuffers timeout; все обязательные CI-checks точного HEAD, включая postgres-integration, test-windows, lint и build, зелёные.
Блокирующее:

  1. internal/query/query.go:2845-2911: buildQualifiedColTypes распознаёт КАК только сразу после обычного ТипИсточника.Имя (aliasPos := i + 3). У поддерживаемой виртуальной таблицы alias стоит после Остатки(), поэтому доказанный date-тип теряется. Публичный пробник query.Compile для День(Ост.Момент) ИЗ РегистрНакопления.События.Остатки() КАК Ост, где Момент — date-измерение, получил strftime('%d', ост.момент) без ob_local_datetime; у границы UTC это снова возвращает не местный день. Нужно учитывать alias после аргументов виртуальной таблицы и добавить регрессионный тест.
    Хвост:
  2. [выброс] Последний коммит содержит Co-Authored-By для ИИ вопреки соглашению CLAUDE.md; отдельную заявку или переписывание уже проверенной истории только ради trailer не предлагаю.
    Вердикт: есть замечания.
    Человеку: это пятый committed-круг после двух прежних решений; нужно решить — довести типизацию alias виртуальной таблицы в этом PR (рекомендую: это тот же типозависимый контракт SQL-функции даты считают UTC/зону СУБД, а те же DSL-функции — time.Local #1243) или явно сузить принимаемый контракт.

@ivanarama

Copy link
Copy Markdown
Owner Author

PromptPilot service marker: REVIEW result publication claimed.

@ivanarama ivanarama added the needs-decision Нужно решение Ивана label Sep 15, 2026
@ivanarama

Copy link
Copy Markdown
Owner Author

PromptPilot service marker: REVIEW result committed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-decision Нужно решение Ивана queue:auto:p1 Inherited automatic queue priority P1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SQL-функции даты считают UTC/зону СУБД, а те же DSL-функции — time.Local

1 participant