kaaveland / eugene

Careful With that Lock, Eugene
MIT License
32 stars 1 forks source link

Consider adding the hints in the json output #27

Closed kaaveland closed 6 months ago

kaaveland commented 6 months ago

We generate hints from src/hints.rs for the markdown report, but we should probably include at least the "code name" for those in the json reports also.

kaaveland commented 6 months ago

The snag on this one is that we use FullSqlStatementLockTrace to render json output, but it's also what's used to generate the hints. In order for it to include hint data, we would probably benefit from introducing a layer of indirection or use a different struct to render json. The quick and dirty hack is to introduce a Vec<HintText> in FullSqlStatementLockTrace and add the hints through mutation after the rest of the struct has been populated with data. Perhaps the best way to approach the problem is to move the hint generation "back", so it queries against SqlStatementTrace which has pub(crate) fields instead of pub fields? That would then also require the hint generation to borrow &TxLockTracer so it could figure out stuff like what locks were held at the start of the statement. 🤔

kaaveland commented 6 months ago

The best design is definitely to push the hint checking to an earlier point in time, before rendering. At that point we have real types anyway, instead of everything being String.

kaaveland commented 6 months ago

Done in #38