Skip to content

fix #1573: добавлен корректный хеш для поиска в таблице по индексу +тест#1575

Merged
EvilBeaver merged 1 commit into
EvilBeaver:developfrom
Mr-Rm:v2/fix-1573
Aug 25, 2025
Merged

fix #1573: добавлен корректный хеш для поиска в таблице по индексу +тест#1575
EvilBeaver merged 1 commit into
EvilBeaver:developfrom
Mr-Rm:v2/fix-1573

Conversation

@Mr-Rm

@Mr-Rm Mr-Rm commented Aug 22, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Bug Fixes

    • Improved consistency of equality and hashing for date and string values, preventing mismatches in collections, caching, lookups, and deduplication.
    • Enhances reliability when searching or indexing data by these types, resulting in more predictable results.
  • Tests

    • Added a unit test verifying search behavior in an indexed column with a specified type to prevent regressions.

@coderabbitai

coderabbitai Bot commented Aug 22, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Introduces 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

Cohort / File(s) Summary
HashCode overrides for value types
src/OneScript.Core/Values/BslDateValue.cs, src/OneScript.Core/Values/BslStringValue.cs
Added public override int GetHashCode() returning the underlying _value's hash code in both classes. No other logic changes.
Unit test: indexed column search
tests/ValueTableIndex.os
Added exported procedure ТестДолжен_ПроверитьПоискПоИндексированнойКолонкеСЗаданнымТипом that builds a table with a typed string column, indexes it, inserts rows, searches via structure, and asserts a single matching result.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my whiskers, hop through code,
Two hashes found their proper road.
A table’s index, neat and tight,
Finds “Стр2” by the book, just right.
Carrots compiled, tests turn green—
Such tidy hops in the build machine! 🥕🐇

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 Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

📥 Commits

Reviewing files that changed from the base of the PR and between da9a017 and 458a559.

📒 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.

Comment thread src/OneScript.Core/Values/BslDateValue.cs
@maximkin7

maximkin7 commented Aug 25, 2025 via email

Copy link
Copy Markdown
Contributor

@EvilBeaver
EvilBeaver merged commit 3581fdd into EvilBeaver:develop Aug 25, 2025
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants