Skip to content

fix(ENGKNOW-2564): Fix dict cache error #59

Merged
gmagnu merged 1 commit into
mainfrom
ENGKNOW-2564-gor-y-option-is-not-working
Jul 22, 2025
Merged

fix(ENGKNOW-2564): Fix dict cache error #59
gmagnu merged 1 commit into
mainfrom
ENGKNOW-2564-gor-y-option-is-not-working

Conversation

@gmagnu
Copy link
Copy Markdown
Contributor

@gmagnu gmagnu commented Jul 8, 2025

Fix dict cache error causing -Y option sometimes not to work.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 8, 2025

Junit Tests - Summary

4 291 tests  +1   4 127 ✅ +1   10m 47s ⏱️ +5s
  456 suites +2     164 💤 ±0 
  456 files   +2       0 ❌ ±0 

Results for commit 778e0e1. ± Comparison against base commit 9e867e7.

@gmagnu gmagnu requested review from Copilot and janeliutw and removed request for Copilot July 21, 2025 22:34
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a dictionary cache error that was causing the -Y option (related to bucket access control) to sometimes not work correctly. The issue was that the cache key generation was inconsistent between different dictionary implementations, leading to cache misses or incorrect cache hits.

  • Standardized cache key generation to include the allowBucketAccess parameter consistently across dictionary implementations
  • Removed the unused isSilentTagFilter parameter from cache key generation in DefaultTableAccessOptimizer
  • Added test coverage to verify dictionary bucket access functionality works correctly

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
DefaultTableAccessOptimizer.java Updated cache key generation to remove unused isSilentTagFilter parameter and only include allowBucketAccess
Dictionary.java Modified cache key to include allowBucketAccess parameter for consistent caching behavior
UTestDictionary.java Added test case to verify dictionary bucket access functionality
BucketManager.java Minor comment fix and made grace period configurable via system property
Comments suppressed due to low confidence (1)

model/src/test/java/org/gorpipe/gor/Dictionary/UTestDictionary.java:421

  • [nitpick] The filename 'testDictionaryWithNoBucketAccess.gord' doesn't match the test method name 'testDictionaryBucketAccess'. Consider renaming to 'testDictionaryBucketAccess.gord' for consistency.
        File gordDict = workDir.newFile("testDictionaryWithNoBucketAccess.gord");

Comment thread gortools/src/main/java/org/gorpipe/gor/manager/BucketManager.java
Comment thread gortools/src/main/java/org/gorpipe/gor/manager/BucketManager.java
@gmagnu gmagnu merged commit e543dd6 into main Jul 22, 2025
11 checks passed
@gmagnu gmagnu deleted the ENGKNOW-2564-gor-y-option-is-not-working branch July 22, 2025 14:56
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