Skip to content
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

tables: Only populate table cache with star-like selects #6513

Merged
merged 2 commits into from Jun 25, 2020

Conversation

theopolis
Copy link
Member

@theopolis theopolis commented Jun 19, 2020

This is a prototype fix for #6510. I wrote the test code without running it.

This has one limitation in that if you were to populate the table cache with:

select * from some_table;

You would expect a cache-hit with:

select some_col from some_table;

But cache-hits are only allowed with star (or similar) selects.

@theopolis theopolis added this to the 4.4.0 milestone Jun 20, 2020
Copy link
Contributor

@muffins muffins left a comment

Choose a reason for hiding this comment

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

Out of curiosity, and as per your statement, do we have documentation about the table caching? Would it be worth adding a note about your caveat to the docs if they exist?

... cache-hits are only allowed with star (or similar) selects.

@amalone-scwx
Copy link

We could support cache-hits with less than '*' selects, as long as no indexed/additional/required columns are used, right? Change isCached() to true in those cases, but setCache() should not persist the results unless it's a star select.

@theopolis
Copy link
Member Author

We could support cache-hits with less than '*' selects

Correct. I was debating adding that feature in this PR but I decided to keep it simple and fix the data inconsistency first.

Would it be worth adding a note about your caveat to the docs if they exist?

I think so, it would be nice to have an "advanced topics" wiki page that covered this and other behind-the-scenes features. Most of that documentation would help us reason about the expectation of code vs. implementation (and point out bugs where expectations are not met!). And some of these features are tunable with flags.

@theopolis theopolis merged commit 25eb7b3 into osquery:master Jun 25, 2020
aikuchin pushed a commit to aikuchin/osquery that referenced this pull request Jul 11, 2023
…0 to master

* commit 'eeee0fb0957f5af983f817c2e6f19c53108d9e09': (83 commits)
  Add additional changelog items (osquery#6523)
  Changelog for 4.4.0 (osquery#6492)
  build: Add Azure tables to specs CMakeLists (osquery#6507)
  CMake: Correct macOS framework linking (osquery#6522)
  tables: Only populate table cache with star-like selects (osquery#6513)
  CMake: Fix and cleanup compile flags (osquery#6521)
  docs: Add note to bump the Homebrew cask (osquery#6519)
  tests: Fix atom_packages, processes, rpm_packages flakiness (osquery#6518)
  bug: Do not use system proxy for AWS local authority (osquery#6512)
  packaging: updating docs on cpack usage to include Chocolatey (osquery#6022)
  bug: Fix typed_row table caching (osquery#6508)
  Implement event batching support for Windows tables (osquery#6280)
  http: Use sync resolve (osquery#6490)
  Add support for basic chassis information (osquery#5282)
  Only emit 'denylist' warning once (osquery#6493)
  docs: Remove references to brew in macOS install (osquery#6494)
  Fix for osquery#5890: Event Format Results and the Kafka Logger (osquery#6449)
  make apt_sources table parsing much more resilient (osquery#6482)
  Make file and hash container columns hidden (osquery#6486)
  Update documentation to use 'allow list' and 'deny list' diction (osquery#6489)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants