Skip to content

Conversation

MeanSquaredError
Copy link
Contributor

@MeanSquaredError MeanSquaredError commented Oct 19, 2025

This PR contains the last (for now) set of ddl2cpp cleanups. It makes the following changes:

  • Changes the code style to be (mostly) PEP 8-compliant. More specifically it makes sure that:
    • Variable and function names use snake_case
    • Class names use CamelCase
    • Private class members are prefixed with an underscore (_)
    • Constant names use SNAKE_CASE.
    • There are two blank lines before/after global classes and functions and one line between class methods and data members.
  • Changes the code to use the new, PEP 8-compliant API of pyparsing. Starting from pyparsing 3.0.0 they have added snake_case methods and variables for their module and classes and have deprecated the old CamelCase methods and variables. ddl2cpp relies on at least one feature in pyparsing 3.0.0 (pyparsing.Located), which means that it requires at least pyparsing 3.0.0 to run, so we can safely switch to their new API. Also if I remember correctly the pyparsing developers said that sooner or later they will start emitting deprecation warnings, so we would have to make the switch anyway.
  • All pyparsing result names and other dictionary keys have been changed from CamelCase to snake_case too. Most of the dictionary keys are also used as object attribute names, so we had to make this change too to be PEP 8-compliant.
  • A few error messages have been clarified. Also all errors and warnings are displayed in a uniform fashion: All uppercase ERROR or WARNING, followed by colon, followed by an error message starting with a capital letter. All error string formatting is done using the modern f-strings, instead of other obsolete formatting methods (e.g. %-style formatting has been replaced by f-strings).
  • More improvements to the custom types processing.
    • Now it checks if the base type (the existing type that is aliased to) is valid and if it is not valid, a meaningful error message is displayed. Also an additional exit code has been added for the case when the custom types CSV file contains invalid base type(s).
    • All the base types use snake_case names, e.g. integral, floating_point, boolean, date_time, etc. This change is mostly in order to make the base type names identical to the data_type names in sqlpp23, i.e. sqlpp::integral, sqlpp::floating_point, etc. However for compatibility purposes, the old CamelCase names for the base types are recognized too, e.g. Integral, FloatingPoint, Boolean, DateTime, etc.
  • The command-line option --path-to-datatype-file has been renamed to --path-to-custom-types. This is pretty much the only really breaking change in this PR. The code and output messages used a bunch of different names for the custom types, e.g. "custom types", "data types", "extended types", which (IMHO) was really confusing to the user. So I chose "custom types" as the (IMHO) most meaningful name and changed all the messages and code to use "custom types" when referring to the user-defined custom types. I also renamed the command-line option the reflect the unified name. I think that not too many people use the old argument name (--path-to-datatype-file), because it only appeared recently, so it shouldn't cause too much problems to the users. If the option name change is too breaking, I can add the old option name as an alias. Or even revert to the old name. But IMHO it is worth keeping the new option name, since it better reflects what the option does.

I did test the new PR and also fixed the tests (both the internal ddl2cpp self-tests and the CMake ones), so hopefully there won't be any breakage this time.

…es use snake_case, class names use CamelCase, private class members are prefixed with an underscore, constant names use SNAKE_CASE.
…ase to snake_case in order to improve PEP 8 compliance.
@rbock
Copy link
Owner

rbock commented Oct 20, 2025

IIUC, the rename of --path-to-datatype-file is the only major functional change?

Can you add a line to docs/change_log.md under the 0.68 heading (maybe I should rename that to next)?

@MeanSquaredError
Copy link
Contributor Author

MeanSquaredError commented Oct 20, 2025

OK, I added the following two lines to docs/change_log.md with description of the changes:

  • sqlpp23-ddl2cpp: command-line option --path-to-datatype-file was renamed to --path-to-custom-types
  • sqlpp23-ddl2cpp: base types in the custom types file should use snake_case, although the old CamelCase is still supported for backwards compatibility

There are also :

  • The slight change in the format of the output messages, e.g. in some places Error: message was changed to ERROR: Message, to make it uniform throughout the program, but that probably shouldn't be listed as a change.
  • The exit error codes were changed slightly: BAD_DATA_TYPE went from 10 to 20, STRANGE_PARSING went from 20 to 30. BAD_CUSTOM_TYPES was added with code 10. But these probably aren't important enough to be recorded, because they were never documented in the first place.

@rbock
Copy link
Owner

rbock commented Oct 20, 2025

Thanks!

Error code changes might be worth documenting.

@MeanSquaredError
Copy link
Contributor Author

MeanSquaredError commented Oct 20, 2025

On a side note - the custom types file should probably be documented better (which is not that difficult to do since it currently has zero documentation :-)
Currently it has the following format:

integral, MYINT, ANOTHER_INT, INTTOO
boolean, my_bool
floating_point, myfloat, ANOTHER_FLOAT
...

The first column contains the base type which should be in snake_case, but could also be CamelCase. The next columns are the custom types, which could be anything and they are case-insensitive.

@MeanSquaredError
Copy link
Contributor Author

OK, I'll add a line for the error code changes.

@MeanSquaredError MeanSquaredError force-pushed the ddl2cpp_cleanup_4 branch 2 times, most recently from 3008f26 to ee37126 Compare October 20, 2025 06:05
@MeanSquaredError
Copy link
Contributor Author

I changed the PR a bit so that BAD_DATA_TYPE and STRANGE_PARSING don't change their codes, just BAD_CUSTOM_TYPES with code 30 is added. Also added a line in docs/change_log.md documenting the new error code.

…id. If not then display a meaningful error message and exit with a specific error code.
…ase names for the base types. This provides compatibility with the old behavior where only CamelCase names were accepted.
@MeanSquaredError
Copy link
Contributor Author

I just pushed a small cosmetic change. It seems that originally somehow I was mistaken and thought that we keep the list of builtin SQL types in uppercase while in fact we keep them in lowercase. So I removed the uppercasing of custom types and left them lowercased as it was originally. Not that it really matters since the DDL parser makes all keyword comparisons case-insensitive.

BTW I just noticed one discrepancy: we keep the list of builtin types in lowecase, but the list of auto-keywords is in uppercase. I won't be fixing that discrepancy in the current PR, since I don't want to add too much noise to the PR.

@MeanSquaredError
Copy link
Contributor Author

AppVeyor timed out so I did a dummy update just to run AppVeyor again.

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.

2 participants