fix #1573: добавлен корректный хеш для поиска в таблице по индексу +тест#1575
Conversation
WalkthroughIntroduces GetHashCode overrides for BslDateValue and BslStringValue to align with their Equals implementations. Adds a new test validating row lookup by an indexed, typed column in a value table. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/OneScript.Core/Values/BslStringValue.cs (1)
48-49: Clarify comparison semantics to prevent future drift.Equals is ordinal (string.Equals), while CompareTo uses CurrentCulture. If that’s intentional per language semantics, add a short comment to document it near one of these members.
Apply this minimal doc hint:
- public override int GetHashCode() => _value.GetHashCode(); + // NOTE: Equals uses ordinal string equality; CompareTo is culture-sensitive. + // Hash must remain consistent with Equals. + public override int GetHashCode() => _value.GetHashCode();tests/ValueTableIndex.os (2)
249-265: Add a negative lookup to harden the test.Also verify that a non-existent key returns zero rows to cover both sides of the predicate.
Apply this small extension at the end of the test:
юТест.ПроверитьРавенство(НайденныеСтроки[0].К1, "Стр2"); -КонецПроцедуры + // Неуспешный поиск по индексу + СтруктураПоиска = Новый Структура("К1", "НеСуществует"); + НайденныеСтроки = Т.НайтиСтроки(СтруктураПоиска); + юТест.ПроверитьРавенство(НайденныеСтроки.Количество(), 0); + +КонецПроцедуры
249-265: Consider a parallel test for Date columns to guard future regressions.Since BslDateValue also received a GetHashCode, a mirrored test with a column of type Дата will protect that path as well. I can open a follow-up PR if helpful.
Example test to add (placement analogous to the string test):
// к issue #1573 (Дата) Процедура ТестДолжен_ПроверитьПоискПоИндексированнойКолонкеДатаСЗаданнымТипом() Экспорт Т = Новый ТаблицаЗначений; Т.Колонки.Добавить("КДата", Новый ОписаниеТипов("Дата")); Т.Индексы.Добавить("КДата"); Д1 = Дата(2025, 8, 20, 12, 30, 0); Д2 = Дата(2025, 8, 21, 12, 30, 0); Д3 = Дата(2025, 8, 22, 12, 30, 0); Т.Добавить().КДата = Д1; Т.Добавить().КДата = Д2; Т.Добавить().КДата = Д3; СтруктураПоиска = Новый Структура("КДата", Д2); НайденныеСтроки = Т.НайтиСтроки(СтруктураПоиска); юТест.ПроверитьРавенство(НайденныеСтроки.Количество(), 1); юТест.ПроверитьРавенство(НайденныеСтроки[0].КДата, Д2); КонецПроцедуры
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/OneScript.Core/Values/BslDateValue.cs(1 hunks)src/OneScript.Core/Values/BslStringValue.cs(1 hunks)tests/ValueTableIndex.os(2 hunks)
🔇 Additional comments (4)
src/OneScript.Core/Values/BslDateValue.cs (1)
48-48: GetHashCode aligned with Equals — this will unblock hash-based index lookups.Returning the underlying DateTime’s hash guarantees consistency with the existing Equals(BslValue) and should fix lookups by typed indexed columns. LGTM.
src/OneScript.Core/Values/BslStringValue.cs (1)
48-49: Hash now consistent with equality — essential for dictionary/index keys.Using string.GetHashCode() aligns with Equals and resolves the typed-index lookup issue for strings. LGTM.
tests/ValueTableIndex.os (2)
28-28: Test registered in the suite — good catch.The new test is added to ПолучитьСписокТестов, so it will run under the harness.
249-265: Positive case for typed, indexed string column — covers the regression.The scenario reproduces the bug and verifies the fix: typed column (Строка), index on К1, and a successful lookup by value.
|
Добрый день!
В период с 25.08.2025 по 07.09.2025 включительно, буду находиться в ежегодном отпуске.
Почта просматриваться не будет.
|
Summary by CodeRabbit
Bug Fixes
Tests