Hi,

Below is a patch that contains a couple suggested documentation changes,
as well as an explanation of how the type system works in type_info.{h,cpp}.

The documentation describes what I was able to reverse engineer from
the code. :-)

Also in type_info.cpp, I changed part of the type lookup table, to match
what I believe is intended: for every entry marked true, there should be
a unique typeid() value. Where there were duplicate typeid() entries,
I left the last one set as true, as that is the one used when you look
at the for loop in the mysql_ti_sql_type_info_lookup constructor.

Feedback welcome,
- Chris


Index: software/mysql++/Wishlist
diff -u software/mysql++/Wishlist:1.1.1.21 software/mysql++/Wishlist:1.3
--- software/mysql++/Wishlist:1.1.1.21 Wed May 11 01:16:59 2005
+++ software/mysql++/Wishlist Wed May 11 01:52:30 2005
@@ -127,3 +127,8 @@
reorganization in 1.7.20: if you move everything from row.cc to
a new row2.h file and include it in a few strategic locations, it
works. But, this is a bogus fix. Find the right one.
+
+ o Document the header layout, and which headers can be included alone,
+ and which can't. Do this to optimize compile times, and allow
+ users to do the same.
+
Index: software/mysql++/lib/null.h
diff -u software/mysql++/lib/null.h:1.1.1.5 software/mysql++/lib/null.h:1.6
--- software/mysql++/lib/null.h:1.1.1.5 Wed May 11 01:17:05 2005
+++ software/mysql++/lib/null.h Wed May 11 02:40:10 2005
@@ -49,7 +49,9 @@
///
/// This template is necessary because there is nothing in the C++ type
/// system with the same semantics as SQL's null. (No, NULL from
-/// stddef.h is not the same!)
+/// stddef.h is not the same!) This template mechanism also
+/// makes the typeid() unique for each C++-equivalent of nullable
+/// SQL field types, which are listed in type_info.cpp.
template <class Type, class Behavior = NullisNull>
class Null {
public:
Index: software/mysql++/lib/type_info.cpp
diff -u software/mysql++/lib/type_info.cpp:1.1.1.3 software/mysql++/lib/type_info.cpp:1.3
--- software/mysql++/lib/type_info.cpp:1.1.1.3 Mon May 2 05:00:10 2005
+++ software/mysql++/lib/type_info.cpp Wed May 11 02:40:10 2005
@@ -14,6 +14,32 @@

typedef string Enum;

+//
+// This array is, somewhat, in the order of the /usr/include/mysql/mysql_com.h
+// enum_field_types list, which is why there are some duplicates. It
+// provides a handy lookup table to translate SQL types (the index of the
+// array) into C++ types (the typeid()).
+//
+// The entire list is duplicated, to provide unique C++ types for non-null
+// and null fields. Null fields are handled by the Null template (null.h).
+//
+// The hard coded number in this list is "base_type", which roughly
+// corresponds to the actual value of enum_field_types. Given a
+// enum_field_type value, and a flag whether it is null and/or unsigned,
+// the mysql_type_info::type(...) function below will give you an index
+// into this array, and thereby convert enum_field_type into a C++ type.
+//
+// Types marked true (the "default" field) are added to a lookup map in
+// the mysql_type_info_lookup class in order to provide reverse lookup
+// of C++ types to SQL types. Put another way, if you take the subset
+// of all items marked true, the typeid() of each item must be unique.
+//
+// Opinion: This seems a very brittle way to code type info, especially
+// considering the sheer infrastructure in these two files
+// (type_info.{h,cpp}). Would be nice if we used the mysql
+// constant names so we would not be vulnerable to number
+// changes in the main mysql C API.
+//
const mysql_type_info::sql_type_info mysql_type_info::types[62] = {
sql_type_info("DECIMAL NOT NULL", typeid(double), 0),
sql_type_info("TINYINT NOT NULL", typeid(signed char), 1, true),
@@ -39,10 +65,10 @@
sql_type_info("CHAR NOT NULL", typeid(string), 21),
sql_type_info("TINYINT UNSIGNED NOT NULL", typeid(unsigned char), 22, true),
sql_type_info("SMALLINT UNSIGNED NOT NULL", typeid(unsigned short int), 23, true),
- sql_type_info("INT UNSIGNED NOT NULL", typeid(unsigned int), 24, true),
- sql_type_info("INT UNSIGNED NOT NULL", typeid(unsigned int), 25, true),
- sql_type_info("INT UNSIGNED NOT NULL", typeid(unsigned int), 26, true),
- sql_type_info("INT UNSIGNED NOT NULL", typeid(unsigned int), 27, true),
+ sql_type_info("INT UNSIGNED NOT NULL", typeid(unsigned int), 24),
+ sql_type_info("INT UNSIGNED NOT NULL", typeid(unsigned int), 25),
+ sql_type_info("INT UNSIGNED NOT NULL", typeid(unsigned int), 26),
+ sql_type_info("INT UNSIGNED NOT NULL", typeid(unsigned int), 27),
sql_type_info("INT UNSIGNED NOT NULL", typeid(unsigned int), 28, true),
sql_type_info("BIGINT UNSIGNED NOT NULL", typeid(ulonglong), 29, true),
sql_type_info("MEDIUMINT UNSIGNED NOT NULL", typeid(unsigned int), 30),
@@ -71,10 +97,10 @@
sql_type_info("CHAR NULL", typeid(Null < string >), 21),
sql_type_info("TINYINT UNSIGNED NULL", typeid(Null < unsigned char >), 22, true),
sql_type_info("SMALLINT UNSIGNED NULL", typeid(Null < unsigned short int >), 23, true),
- sql_type_info("INT UNSIGNED NULL", typeid(Null < unsigned int >), 24, true),
- sql_type_info("INT UNSIGNED NULL", typeid(Null < unsigned int >), 25, true),
- sql_type_info("INT UNSIGNED NULL", typeid(Null < unsigned int >), 26, true),
- sql_type_info("INT UNSIGNED NULL", typeid(Null < unsigned int >), 27, true),
+ sql_type_info("INT UNSIGNED NULL", typeid(Null < unsigned int >), 24),
+ sql_type_info("INT UNSIGNED NULL", typeid(Null < unsigned int >), 25),
+ sql_type_info("INT UNSIGNED NULL", typeid(Null < unsigned int >), 26),
+ sql_type_info("INT UNSIGNED NULL", typeid(Null < unsigned int >), 27),
sql_type_info("INT UNSIGNED NULL", typeid(Null < unsigned int >), 28, true),
sql_type_info("BIGINT UNSIGNED NULL", typeid(Null < ulonglong >), 29, true),
sql_type_info("MEDIUMINT UNSIGNED NULL", typeid(Null < unsigned int >), 30),


--
MySQL++ Mailing List
For list archives: http://lists.mysql.com/plusplus
To unsubscribe: http://lists.mysql.com/plusplus?unsu...ie.nctu.edu.tw

Chris Frey wrote:

I was able to rewrite this to be a lot clearer, I think. Some of it was
just straight rewriting, but I also moved a lot of it to more
appropraite places, so only the salient bits remain in type_info.cpp.
The "opinion" block is now a Wishlist item, and the explanation of
type()'s inner workings became the seed of a Doxygen comment in type_info.h.

I didn't study it deeply enough to know whether this is the right thing,
so I've decided to trust that you know what you're doing here.

Have you read Lakos's "Large Scale C++" book (probably got the title
wrong)? Are you proposing optimization of that sort?

The thing that bugs me about mysql++.h now is that it includes every
header, most of them in straight alphabetical order. This is certainly
not the most efficient way. There are two ways we can improve this:

- The obvious way is to stare at the Doxygen-generated include graphs
long enough that we learn which #includes we can pare out of mysql++.h,
and what reorderings would help, if any. The problem with this is, it's
brittle. If we depend on one "leaf" header to #include a particular
file and so don't include it from mysql++ any more, things will break if
we find later that the leaf header doesn't actually need to #include the
other one, so we remove it.

- A smarter way to go is to wrap each #include with the idempotency
guard for that header, so it is never included from mysql++.h if a
previous header already included it. You can push this concept further
by doing the same in the other headers, but just doing it at the top
level will probably save a significant chunk of time, and there's no
sense cluttering the code for small gains. Unless I'm very wrong, the
case we want to optimize for is end users building their own programs,
not build time for MySQL++. Putting mysql++.h in order should recover
most of the wasted time for the end-user case.

No objection. Accepted.

--
MySQL++ Mailing List
For list archives: http://lists.mysql.com/plusplus
To unsubscribe: http://lists.mysql.com/plusplus?unsu...ie.nctu.edu.tw

Warren Young wrote:

I just tried this, and it doesn't make a whit of difference when
building the examples.



--
MySQL++ Mailing List
For list archives: http://lists.mysql.com/plusplus
To unsubscribe: http://lists.mysql.com/plusplus?unsu...ie.nctu.edu.tw

On Thu, May 12, 2005 at 03:54:14PM -0600, Warren Young wrote:
Cool.

If you can hold off the next release for a couple days, I can be even
more definite I'm right, and can post a little test to prove it.

I only changed the initializers for the "_default" member. This variable
is only used in the lookup class, in its constructor (grep proves this).
The for loop cycles through the entire list, and adds any item marked
true in _default to the lookup std::map. Since there are some entries
with identical typeid() info, and set to true, the last one will end up
in the lookup table. That's why I left the last item set to true, and
disabled the rest.


Afraid I haven't read it. The optimization I'm proposing is just based
on my experience. Including everything is often not necessary.
If the user only needs connection and query objects, some headers may
not be needed.


This is more like what I had in mind, but it seems you have already
tried this and nothing changed. I don't fully know the whole inter-
dependencyes of the headers, so maybe using Query does pull everything
in anyway, but I figured it might be worth a look.

The other change would be to replace any unnecessary #includes with
forward declarations. You probably did this when you did the
Giant Header Reorg, so maybe none of this is that useful. :-)

I just notice the examples take a while to compile on my system (PII) and
thought something could be done. If I do manage an increase, I'll just
post the patch when I have something, and stop talking about it for now.
You don't have to add it to the wishlist, since I'm probably the only
guy with a slow enough system that cares, and will remember myself. :-)

- Chris


--
MySQL++ Mailing List
For list archives: http://lists.mysql.com/plusplus
To unsubscribe: http://lists.mysql.com/plusplus?unsu...ie.nctu.edu.tw

On Fri, May 13, 2005 at 04:17:01AM -0400, Chris Frey wrote:
Below is a test patch only, not intended for inclusion with the distro.
All it does is dump the contents of the lookups map to stdout, displaying
the C++ type name, and the corresponding mysql enum code.

I ran it twice: once with the type_info patch from earlier, and one with
the original 'true' settings. The dump was identical for both test runs,
proving that the earlier patch makes no change to the underlying behaviour.

Here is the dump... kinda messy with all the C++ names, but included for
completeness.

First: N7mysqlpp4NullIyNS_10NullisNullEEE Second: 60
First: N7mysqlpp4NullIjNS_10NullisNullEEE Second: 59
First: N7mysqlpp4NullItNS_10NullisNullEEE Second: 54
First: N7mysqlpp4NullIhNS_10NullisNullEEE Second: 53
First: N7mysqlpp4NullINS_3SetISsEENS_10NullisNullEEE Second: 45
First: N7mysqlpp4NullISsNS_10NullisNullEEE Second: 50
First: N7mysqlpp4NullINS_8DateTimeENS_10NullisNullEEE Second: 43
First: N7mysqlpp4NullINS_4DateENS_10NullisNullEEE Second: 41
First: N7mysqlpp4NullIxNS_10NullisNullEEE Second: 39
First: N7mysqlpp4NullINS_4TimeENS_10NullisNullEEE Second: 42
First: N7mysqlpp4NullIfNS_10NullisNullEEE Second: 35
First: N7mysqlpp4NullIiNS_10NullisNullEEE Second: 34
First: N7mysqlpp4NullIsNS_10NullisNullEEE Second: 33
First: N7mysqlpp4NullIaNS_10NullisNullEEE Second: 32
First: N7mysqlpp4NullIdNS_10NullisNullEEE Second: 36
First: N7mysqlpp3SetISsEE Second: 14
First: Ss Second: 19
First: N7mysqlpp4DateE Second: 10
First: N7mysqlpp4TimeE Second: 11
First: N7mysqlpp8DateTimeE Second: 12
First: a Second: 1
First: h Second: 22
First: s Second: 2
First: t Second: 23
First: i Second: 3
First: j Second: 28
First: x Second: 8
First: y Second: 29
First: f Second: 4
First: d Second: 5


And here is the patch including the test code.

- Chris



Index: software/mysql++/examples/Makefile.am
diff -u software/mysql++/examples/Makefile.am:1.2 software/mysql++/examples/Makefile.am:1.3
--- software/mysql++/examples/Makefile.am:1.2 Wed May 11 02:08:37 2005
+++ software/mysql++/examples/Makefile.am Wed May 18 10:40:03 2005
@@ -5,12 +5,16 @@

EXTRA_DIST = *.vcproj *.bpf *.bpr *.bpg Makefile.simple

-noinst_PROGRAMS = resetdb simple1 custom1 custom2 custom3 custom4 \
+noinst_PROGRAMS = dump_types \
+ resetdb simple1 custom1 custom2 custom3 custom4 \
custom5 \
complic1 fieldinf1 dbinfo updel load_file cgi_image

noinst_HEADERS = util.h

+dump_types_SOURCES = dump_types.cpp
+dump_types_DEPENDENCIES = $(MYSQLPP_LIB)
+
simple1_SOURCES = simple1.cpp util.cpp
simple1_DEPENDENCIES = $(MYSQLPP_LIB)

Index: software/mysql++/examples/dump_types.cpp
diff -u /dev/null software/mysql++/examples/dump_types.cpp:1.1
--- /dev/null Wed May 18 10:42:11 2005
+++ software/mysql++/examples/dump_types.cpp Wed May 18 10:40:03 2005
@@ -0,0 +1,9 @@
+#include <iostream>
+#include <mysql++.h>
+
+int main()
+{
+ mysqlpp::mysql_type_info::dump_lookups(std::cout);
+ return 0;
+}
+
Index: software/mysql++/lib/type_info.cpp
diff -u software/mysql++/lib/type_info.cpp:1.3 software/mysql++/lib/type_info.cpp:1.4
--- software/mysql++/lib/type_info.cpp:1.3 Wed May 11 02:40:10 2005
+++ software/mysql++/lib/type_info.cpp Wed May 18 10:40:03 2005
@@ -174,5 +174,15 @@
return false;
}

+void mysql_type_info::dump_lookups(std:stream &os)
+{
+ std::map<const std::type_info *, unsigned char, type_info_cmp>::const_iterator i = lookups._map.begin();
+ for( ; i != lookups._map.end(); i++ ) {
+ os << "First: " << i->first->name()
+ << " Second: " << static_cast<unsigned int> (i->second)
+ << endl;
+ }
+}
+
} // end namespace mysqlpp

Index: software/mysql++/lib/type_info.h
diff -u software/mysql++/lib/type_info.h:1.4 software/mysql++/lib/type_info.h:1.5
--- software/mysql++/lib/type_info.h:1.4 Mon May 2 06:25:53 2005
+++ software/mysql++/lib/type_info.h Wed May 18 10:40:03 2005
@@ -170,6 +170,8 @@
/// Returns true if the SQL ID of this type is lower than that of
/// another. Used by mysqlpp::type_info_cmp when comparing types.
bool before(mysql_type_info &b) { return num < b.num; }
+
+ static void dump_lookups(std:stream &os);
};

inline const mysql_type_info::sql_type_info& mysql_type_info::deref() const


--
MySQL++ Mailing List
For list archives: http://lists.mysql.com/plusplus
To unsubscribe: http://lists.mysql.com/plusplus?unsu...ie.nctu.edu.tw

Chris Frey wrote:
Good enough for me.

In fact, I hope you did this mostly for yourself, as it was good enough
for me before. I hope you didn't think I required this.

--
MySQL++ Mailing List
For list archives: http://lists.mysql.com/plusplus
To unsubscribe: http://lists.mysql.com/plusplus?unsu...ie.nctu.edu.tw

On Wed, May 18, 2005 at 11:02:36AM -0600, Warren Young wrote:
I wanted to make sure too. :-)

- Chris


--
MySQL++ Mailing List
For list archives: http://lists.mysql.com/plusplus
To unsubscribe: http://lists.mysql.com/plusplus?unsu...ie.nctu.edu.tw