commit 6bc49c79da66449285dc26e24d3a1c4986124c1b
author Lalit Maganti <lalitm@google.com> Wed Oct 16 10:44:52 2019 +0100
committer Lalit Maganti <lalitm@google.com> Wed Oct 16 10:44:52 2019 +0100
tree c3c9791ee17288c6efd03f81b9b08205d62c031a
parent 0e17af90d4c9b2e6514eaf9c44dcf77a74be8a27

trace_processor: fix support for optional<stringid> columns

We didn't have proper support for optional<stringid> columns because
we expected all stringid columns to directly use stringid as stringpool
has a special id (0) reserved for null strings.

However, this is confusing for end users as it's not obvious why they
can't use it in the macro tables. Moreover, the current code silently
failed without any warnings and only crashed in debug.

We don't want to have nullability at the sparsevector level because it
would require an additional layer of null checking in all the string
filtering code

Therefore, there are two options for fixing this:
1. We redirect base::nullopt -> null string id at insert time. This
   allows the rest of the code to work as is and doesn't add an extra
   overhead at query time.
2. We add a static_assert to ban optional<stringid> and educate
   users to use stringid instead. This makes the internal structure
   of macro tables clearer than option 1 but is surprising and forces
   users to obscure intent of their column types.

This CL implements option 1 with the note that, one day, we may be able
to get rid of the nullability at the stringpool level; in this case, we
can remove this hack and treat strings just the same as we treat
integers.

Bug: 1142720466
Change-Id: Ied75ddfa6ba9ada16727612b60bcd7bba68dd011