Skip to content

[IMP] util/pg: cache column_exists for common columns #285

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

Closed

Conversation

aj-fuentes
Copy link
Contributor

In many cases we know that columns must exist, it's better to cache them
to avoid thousands of small queries.

@robodoo
Copy link
Contributor

robodoo commented Jun 17, 2025

Pull request status dashboard

@aj-fuentes
Copy link
Contributor Author

upgradeci retry with always hr* _hr

@aj-fuentes aj-fuentes requested a review from KangOl June 17, 2025 14:46
@aj-fuentes aj-fuentes force-pushed the master-imp_cache_column_exists-afu branch from 4d381c4 to aff56cc Compare June 18, 2025 09:16
src/util/pg.py Outdated
Comment on lines 486 to 499
def _forced_cache(cache):
def decorator(f):
@functools.wraps(f)
def wrapper(cr, *args, **kwargs):
if not kwargs and tuple(args) in cache:
return cache[tuple(args)]
return f(cr, *args, **kwargs)

return wrapper

return decorator


@_forced_cache(_COLUMNS)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the decorator is needed. It can be inline into the function directly.

def column_exists(cr, table, column):
    if (table, column) in _COLUMNS:
        return _COLUMNS[(table, column)]
    return _column_info(cr, table, column) is not None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, originally I was using it in get_value_or_en_translation. It proved not as useful. I'm still working on this, I will simplify as much as possible before the final version. There are still some other columns being checked more than 70 times in the local test I'm doing.

@KangOl
Copy link
Contributor

KangOl commented Jun 18, 2025

upgradeci retry with always only hr* *_hr* in all versions

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
In many cases we know that columns must exist, it's better to cache them
to avoid thousands of small queries.
@aj-fuentes aj-fuentes force-pushed the master-imp_cache_column_exists-afu branch from aff56cc to 66473d6 Compare June 18, 2025 15:37
Copy link
Contributor

@KangOl KangOl left a comment

Choose a reason for hiding this comment

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

robodoo pushed a commit that referenced this pull request Jun 24, 2025
In many cases we know that columns must exist, it's better to cache them
to avoid thousands of small queries.

closes #285

Signed-off-by: Christophe Simonis (chs) <[email protected]>
robodoo pushed a commit that referenced this pull request Jun 24, 2025
In many cases we know that columns must exist, it's better to cache them
to avoid thousands of small queries.

closes #285

Signed-off-by: Christophe Simonis (chs) <[email protected]>
robodoo pushed a commit that referenced this pull request Jun 24, 2025
In many cases we know that columns must exist, it's better to cache them
to avoid thousands of small queries.

closes #285

Signed-off-by: Christophe Simonis (chs) <[email protected]>
robodoo pushed a commit that referenced this pull request Jun 24, 2025
In many cases we know that columns must exist, it's better to cache them
to avoid thousands of small queries.

closes #285

Signed-off-by: Christophe Simonis (chs) <[email protected]>
robodoo pushed a commit that referenced this pull request Jun 24, 2025
In many cases we know that columns must exist, it's better to cache them
to avoid thousands of small queries.

closes #285

Signed-off-by: Christophe Simonis (chs) <[email protected]>
robodoo pushed a commit that referenced this pull request Jun 24, 2025
In many cases we know that columns must exist, it's better to cache them
to avoid thousands of small queries.

closes #285

Signed-off-by: Christophe Simonis (chs) <[email protected]>
@robodoo robodoo closed this in b977faa Jun 24, 2025
@robodoo robodoo added the 18.5 label Jun 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants