From ba5f70225769e5fa388e8beab96eba2c218eb012 Mon Sep 17 00:00:00 2001 From: Tomasz Sowa Date: Wed, 18 Sep 2019 12:29:20 +0000 Subject: [PATCH] fixed: in 'left join' statements there were not table indices used added: now we set flag save_mode = DO_NOTHING_ON_SAVE for objects for which all fields from a database result set are null git-svn-id: svn://ttmath.org/publicrep/morm/trunk@1210 e52654a7-88a9-db11-a3e9-0013d4bc506e --- samples/Makefile.dep | 7 +-- samples/sample01.h | 20 ++++----- src/Makefile.dep | 11 ++--- src/cursor.h | 16 ++++--- src/model.cpp | 25 +++++++++++ src/model.h | 80 +++++++++++++++++++---------------- src/modelenv.h | 3 ++ src/postgresqlqueryresult.cpp | 18 +++++++- src/postgresqlqueryresult.h | 2 + src/queryresult.cpp | 5 +++ src/queryresult.h | 1 + 11 files changed, 126 insertions(+), 62 deletions(-) diff --git a/samples/Makefile.dep b/samples/Makefile.dep index 0c04d27..d56c775 100644 --- a/samples/Makefile.dep +++ b/samples/Makefile.dep @@ -15,9 +15,10 @@ main.o: ../../morm/src/dbexpression.h ../../morm/src/baseexpression.h main.o: ../../morm/src/modelenv.h ../../morm/src/modeldata.h main.o: ../../morm/src/cursorhelper.h ../../morm/src/finderhelper.h main.o: ../../morm/src/fieldvaluehelper.h ../../morm/src/flatexpression.h -main.o: ../../morm/src/postgresqlqueryresult.h ../../morm/src/finder.h -main.o: ../../pikotools/utf8/utf8.h ../../morm/src/cursor.h -main.o: ../../morm/src/jsonexpression.h ../../morm/src/postgresqlexpression.h +main.o: ../../morm/src/finder.h ../../pikotools/utf8/utf8.h +main.o: ../../morm/src/cursor.h ../../morm/src/jsonexpression.h +main.o: ../../morm/src/postgresqlexpression.h main.o: ../../morm/src/dochtmlexpression.h ../../morm/src/jsonconnector.h main.o: ../../morm/src/postgresqlconnector.h +main.o: ../../morm/src/postgresqlqueryresult.h main.o: ../../morm/src/dochtmlconnector.h person.h language.h attachment.h diff --git a/samples/sample01.h b/samples/sample01.h index ad9ee30..93a3fd8 100644 --- a/samples/sample01.h +++ b/samples/sample01.h @@ -65,17 +65,17 @@ void make() morm::Finder finder(model_connector); - //Person p = finder.use_table_prefix(false).select().where().eq(L"id", 110).get(); + Person p = finder.use_table_prefix(false).select().where().eq(L"id", 120).get(); - Person p = finder.prepare_to_select().use_table_prefix(true).raw("select person.id as \"person.id\", person.first_name as \"person.first_name\", person.last_name as \"person.last_name\", person.email as \"person.email\", " - "language.id as \"language.id\", language.english_name as \"language.english_name\", language.local_name as \"language.local_name\", language.code_str as \"language.code_str\", language.code_int as \"language.code_int\", " - "attachment.id as \"attachment.id\", attachment.person_id as \"attachment.person_id\", attachment.name as \"attachment.name\", attachment.content as \"attachment.content\", attachment.some_flags as \"attachment.some_flags\", attachment.created_date as \"attachment.created_date\"," - "language2.id as \"language2.id\", language2.english_name as \"language2.english_name\", language2.local_name as \"language2.local_name\", language2.code_str as \"language2.code_str\", language2.code_int as \"language2.code_int\"" - "FROM public.person AS person " - "LEFT JOIN public.language AS language ON person.language_id = language.id " - "LEFT JOIN public.attachment AS attachment ON person.id = attachment.person_id " - "LEFT JOIN public.language AS language2 ON attachment.language_id = language.id " - "where person.id=110").get(); +// Person p = finder.prepare_to_select().use_table_prefix(true).raw("select person.id as \"person.id\", person.first_name as \"person.first_name\", person.last_name as \"person.last_name\", person.email as \"person.email\", " +// "language.id as \"language.id\", language.english_name as \"language.english_name\", language.local_name as \"language.local_name\", language.code_str as \"language.code_str\", language.code_int as \"language.code_int\", " +// "attachment.id as \"attachment.id\", attachment.person_id as \"attachment.person_id\", attachment.name as \"attachment.name\", attachment.content as \"attachment.content\", attachment.some_flags as \"attachment.some_flags\", attachment.created_date as \"attachment.created_date\"," +// "language2.id as \"language2.id\", language2.english_name as \"language2.english_name\", language2.local_name as \"language2.local_name\", language2.code_str as \"language2.code_str\", language2.code_int as \"language2.code_int\"" +// "FROM public.person AS person " +// "LEFT JOIN public.language AS language ON person.language_id = language.id " +// "LEFT JOIN public.attachment AS attachment ON person.id = attachment.person_id " +// "LEFT JOIN public.language AS language2 ON attachment.language_id = language2.id " +// "where person.id=120").get(); std::string str; p.to_text(str, true, true); diff --git a/src/Makefile.dep b/src/Makefile.dep index ec7cca1..6ec9950 100644 --- a/src/Makefile.dep +++ b/src/Makefile.dep @@ -11,8 +11,7 @@ baseexpression.o: modeldata.h cursorhelper.h queryresult.h baseexpression.o: ../../pikotools/log/log.h ../../pikotools/log/filelog.h baseexpression.o: finderhelper.h fieldvaluehelper.h model.h modelconnector.h baseexpression.o: clearer.h dbconnector.h flatconnector.h dbexpression.h -baseexpression.o: flatexpression.h postgresqlqueryresult.h -baseexpression.o: ../../pikotools/utf8/utf8.h +baseexpression.o: flatexpression.h ../../pikotools/utf8/utf8.h clearer.o: clearer.h ../../pikotools/date/date.h clearer.o: ../../pikotools/convert/inttostr.h model.h clearer.o: ../../pikotools/textstream/textstream.h @@ -22,7 +21,7 @@ clearer.o: ../../pikotools/textstream/types.h modelconnector.h dbconnector.h clearer.o: ../../pikotools/log/log.h ../../pikotools/log/filelog.h clearer.o: queryresult.h flatconnector.h dbexpression.h baseexpression.h clearer.o: morm_types.h modelenv.h modeldata.h cursorhelper.h finderhelper.h -clearer.o: fieldvaluehelper.h flatexpression.h postgresqlqueryresult.h +clearer.o: fieldvaluehelper.h flatexpression.h dbconnector.o: dbconnector.h ../../pikotools/textstream/textstream.h dbconnector.o: ../../pikotools/space/space.h dbconnector.o: ../../pikotools/textstream/types.h ../../pikotools/date/date.h @@ -33,8 +32,7 @@ dbconnector.o: ../../pikotools/log/filelog.h queryresult.h dbexpression.h dbconnector.o: baseexpression.h morm_types.h modelenv.h modeldata.h dbconnector.o: cursorhelper.h finderhelper.h fieldvaluehelper.h model.h dbconnector.o: modelconnector.h clearer.h flatconnector.h flatexpression.h -dbconnector.o: postgresqlqueryresult.h ../../pikotools/utf8/utf8.h -dbconnector.o: ../../pikotools/convert/convert.h +dbconnector.o: ../../pikotools/utf8/utf8.h ../../pikotools/convert/convert.h dbconnector.o: ../../pikotools/convert/inttostr.h dbconnector.o: ../../pikotools/convert/patternreplacer.h dbconnector.o: ../../pikotools/convert/strtoint.h @@ -84,7 +82,7 @@ flatconnector.o: baseexpression.h morm_types.h modelenv.h modeldata.h flatconnector.o: cursorhelper.h queryresult.h ../../pikotools/log/log.h flatconnector.o: ../../pikotools/log/filelog.h finderhelper.h flatconnector.o: fieldvaluehelper.h model.h modelconnector.h clearer.h -flatconnector.o: dbconnector.h dbexpression.h postgresqlqueryresult.h +flatconnector.o: dbconnector.h dbexpression.h flatexpression.o: flatexpression.h baseexpression.h flatexpression.o: ../../pikotools/textstream/textstream.h flatexpression.o: ../../pikotools/space/space.h @@ -128,7 +126,6 @@ model.o: dbconnector.h ../../pikotools/log/log.h model.o: ../../pikotools/log/filelog.h queryresult.h flatconnector.h model.o: dbexpression.h baseexpression.h morm_types.h modelenv.h modeldata.h model.o: cursorhelper.h finderhelper.h fieldvaluehelper.h flatexpression.h -model.o: postgresqlqueryresult.h modelconnector.o: modelconnector.h clearer.h ../../pikotools/date/date.h modelconnector.o: ../../pikotools/convert/inttostr.h dbconnector.h modelconnector.o: ../../pikotools/textstream/textstream.h diff --git a/src/cursor.h b/src/cursor.h index 151b191..ff49c43 100644 --- a/src/cursor.h +++ b/src/cursor.h @@ -203,9 +203,13 @@ public: { if( query_result->cur_row < query_result->result_rows ) { - result.set_save_mode(Model::DO_UPDATE_ON_SAVE); // IMPROVE ME check if there is a primary key result.map_values_from_query(); - result.after_select(); + + if( result.found() ) + { + result.after_select(); + } + query_result->cur_row += 1; } else @@ -320,7 +324,6 @@ protected: { result.emplace_back(); // it returns a reference from c++17 ModelClass & added_model = result.back(); - ModelEnv model_env_local; try @@ -338,7 +341,6 @@ protected: added_model.model_env = &model_env_local; added_model.model_env->cursor_helper = &cursor_helper; added_model.model_env->finder_helper = &finder_helper; - added_model.set_save_mode(Model::DO_UPDATE_ON_SAVE); // IMPROVE ME check if there is a primary key added_model.model_env->model_data = model_data; if( !cursor_helper.has_autogenerated_select && cursor_helper.use_table_prefix_for_fetching_values ) @@ -348,7 +350,11 @@ protected: added_model.before_select(); added_model.map_values_from_query(); - added_model.after_select(); + + if( added_model.found() ) + { + added_model.after_select(); + } } catch(...) { diff --git a/src/model.cpp b/src/model.cpp index deae7f2..db52ef0 100644 --- a/src/model.cpp +++ b/src/model.cpp @@ -622,8 +622,18 @@ void Model::map_values_from_query() if( model_env ) { model_env->model_connector_mode = MORM_MODEL_CONNECTOR_MODE_READING_VALUE_FROM_DB_RESULTSET; + model_env->all_fields_are_null = true; map_fields(); model_env->model_connector_mode = MORM_MODEL_CONNECTOR_MODE_NONE; + + if( model_env->all_fields_are_null ) + { + save_mode = DO_NOTHING_ON_SAVE; + } + else + { + save_mode = DO_UPDATE_ON_SAVE; + } } } @@ -774,5 +784,20 @@ void Model::prepare_table_names(bool prepare_table_index) } } + +void Model::put_table_name_with_index(PT::TextStream & str) +{ + if( model_env ) + { + str << model_env->table_name_short; + + if( model_env->table_index > 1 ) + { + str << model_env->table_index; + } + } +} + + } // namespace diff --git a/src/model.h b/src/model.h index 272ede2..aeec000 100644 --- a/src/model.h +++ b/src/model.h @@ -822,13 +822,11 @@ protected: if( model_env && field_model.model_env && model_env->finder_helper ) { model_env->finder_helper->foreign_keys.clear(); + PT::TextStream & join_tables_str = model_env->finder_helper->join_tables_str; field_model.model_env->table_index = model_env->finder_helper->add_join_table(field_model.model_env->table_name_short); - model_env->finder_helper->join_tables_str << "LEFT JOIN " << field_model.model_env->table_name - << " AS " << field_model.model_env->table_name_short; - - if( field_model.model_env->table_index > 1 ) - model_env->finder_helper->join_tables_str << field_model.model_env->table_index; + join_tables_str << "LEFT JOIN " << field_model.model_env->table_name << " AS "; + field_model.put_table_name_with_index(join_tables_str); int expr_work_mode = db_expression->get_work_mode(); int expr_output_type = db_expression->get_output_type(); @@ -842,12 +840,16 @@ protected: { field_model.map_fields(); - model_env->finder_helper->join_tables_str << " ON " << model_env->table_name_short << '.' - << db_field_name << " = " << field_model.model_env->table_name_short << '.'; + join_tables_str << " ON "; + put_table_name_with_index(join_tables_str); + join_tables_str << '.' << db_field_name << " = "; + field_model.put_table_name_with_index(join_tables_str); + join_tables_str << '.'; + // IMPROVE ME at the moment support only for foreign keys consisting of only one column if( model_env->finder_helper->foreign_keys.size() == 1 ) { - model_env->finder_helper->join_tables_str << model_env->finder_helper->foreign_keys.front(); + join_tables_str << model_env->finder_helper->foreign_keys.front(); } } else @@ -856,17 +858,22 @@ protected: map_fields(); // map_fields() will set field_model.model_env to null field_model.model_env = old_model_env; - model_env->finder_helper->join_tables_str << " ON " << model_env->table_name_short << '.'; + join_tables_str << " ON "; + put_table_name_with_index(join_tables_str); + join_tables_str << '.'; + // IMPROVE ME at the moment support only for foreign keys consisting of only one column if( model_env->finder_helper->foreign_keys.size() == 1 ) { - model_env->finder_helper->join_tables_str << model_env->finder_helper->foreign_keys.front(); + join_tables_str << model_env->finder_helper->foreign_keys.front(); } - model_env->finder_helper->join_tables_str << " = " << field_model.model_env->table_name_short << '.' << db_field_name; + join_tables_str << " = "; + field_model.put_table_name_with_index(join_tables_str); + join_tables_str << '.' << db_field_name; } - model_env->finder_helper->join_tables_str << ' '; + join_tables_str << ' '; db_expression->set_work_mode(expr_work_mode); db_expression->set_output_type(expr_output_type); @@ -1012,14 +1019,13 @@ protected: field_model.prepare_table_names(); } - // IMPROVE ME - // we need to test if the object is actually defined, test nulls on primary key? - // or check every field whether is it null? - field_model.before_select(); - field_model.set_save_mode(Model::DO_UPDATE_ON_SAVE); // IMPROVE ME check if there is a primary key field_model.map_values_from_query(); - field_model.after_select(); + + if( field_model.found() ) + { + field_model.after_select(); + } } } } @@ -1264,15 +1270,16 @@ protected: template void get_value_by_field_index(int field_index, FieldValue & field_value) { - if( model_env->cursor_helper && model_env->cursor_helper->query_result ) + DbConnector * db_connector = model_connector->get_db_connector(); + + if( db_connector && model_env->cursor_helper && model_env->cursor_helper->query_result ) { - const char * val_str = model_env->cursor_helper->query_result->get_field_string_value(field_index); - - if( val_str ) + if( !model_env->cursor_helper->query_result->is_null(field_index) ) { - DbConnector * db_connector = model_connector->get_db_connector(); + model_env->all_fields_are_null = false; + const char * val_str = model_env->cursor_helper->query_result->get_field_string_value(field_index); - if( db_connector ) + if( val_str ) { db_connector->get_value(val_str, field_value); } @@ -1288,7 +1295,7 @@ protected: if( db_connector && model_env->cursor_helper && model_env->cursor_helper->query_result ) { - const char * val_str = nullptr; + int column_index = -1; if( model_env->cursor_helper->use_table_prefix_for_fetching_values && model_env->finder_helper ) { @@ -1297,26 +1304,26 @@ protected: std::wstring table_field_name; PT::TextStream table_field_name_str; - table_field_name_str = model_env->table_name_short; - - if( model_env->table_index > 1 ) - { - table_field_name_str << model_env->table_index; - } - + put_table_name_with_index(table_field_name_str); table_field_name_str << '.'; table_field_name_str << field_name; table_field_name_str.to_string(table_field_name); - val_str = model_env->cursor_helper->query_result->get_field_string_value(table_field_name.c_str()); + column_index = model_env->cursor_helper->query_result->get_column_index(table_field_name.c_str()); } else { - val_str = model_env->cursor_helper->query_result->get_field_string_value(field_name); + column_index = model_env->cursor_helper->query_result->get_column_index(field_name); } - if( val_str ) + if( column_index != -1 && !model_env->cursor_helper->query_result->is_null(column_index) ) { - db_connector->get_value(val_str, field_value); + model_env->all_fields_are_null = false; + const char * val_str = model_env->cursor_helper->query_result->get_field_string_value(column_index); + + if( val_str ) + { + db_connector->get_value(val_str, field_value); + } } } } @@ -1475,6 +1482,7 @@ protected: virtual bool is_empty_field(const wchar_t * value); virtual bool is_the_same_field(const wchar_t * field1, const wchar_t * field2); virtual void prepare_table_names(bool prepare_table_index = true); + virtual void put_table_name_with_index(PT::TextStream & str); template friend class Finder; template friend class Cursor; diff --git a/src/modelenv.h b/src/modelenv.h index 160f9d3..3f9e26c 100644 --- a/src/modelenv.h +++ b/src/modelenv.h @@ -66,6 +66,7 @@ public: PT::TextStream table_name_short; int table_index; int field_index; + bool all_fields_are_null; std::vector * set_field_name_helper; std::vector * field_value_helper_tab; @@ -94,6 +95,7 @@ public: set_field_name_helper = e.set_field_name_helper; field_value_helper_tab = e.field_value_helper_tab; field_index = e.field_index; + all_fields_are_null = e.all_fields_are_null; // table_name and table_name_short don't have to bo copied } @@ -125,6 +127,7 @@ public: set_field_name_helper = nullptr; field_value_helper_tab = nullptr; field_index = 0; + all_fields_are_null = false; } diff --git a/src/postgresqlqueryresult.cpp b/src/postgresqlqueryresult.cpp index fffff4c..4391d3e 100644 --- a/src/postgresqlqueryresult.cpp +++ b/src/postgresqlqueryresult.cpp @@ -71,7 +71,7 @@ const char * PostgreSQLQueryResult::get_field_string_value(int column_index) { const char * value_str = nullptr; - if( column_index >= 0 && (size_t)column_index < result_cols ) + if( psql_result && column_index >= 0 && (size_t)column_index < result_cols ) { if( cur_row < result_rows ) { @@ -120,6 +120,22 @@ int PostgreSQLQueryResult::get_column_index(const char * column_name) } +bool PostgreSQLQueryResult::is_null(int column_index) +{ + bool res = false; + + if( column_index >= 0 && (size_t)column_index < result_cols ) + { + if( cur_row < result_rows ) + { + res = is_null(cur_row, column_index); + } + } + + return res; +} + + const char * PostgreSQLQueryResult::get_value_from_result(int row, int col) { diff --git a/src/postgresqlqueryresult.h b/src/postgresqlqueryresult.h index 638bebf..ca80482 100644 --- a/src/postgresqlqueryresult.h +++ b/src/postgresqlqueryresult.h @@ -61,6 +61,8 @@ struct PostgreSQLQueryResult : public QueryResult const char * get_field_string_value(const char * column_name); int get_column_index(const char * column_name); + bool is_null(int column_index); + const char * get_value_from_result(int row, int col); int get_value_length(int row, int col); diff --git a/src/queryresult.cpp b/src/queryresult.cpp index da2c7b7..cdac798 100644 --- a/src/queryresult.cpp +++ b/src/queryresult.cpp @@ -103,6 +103,11 @@ int QueryResult::get_column_index(const wchar_t * column_name) } +bool QueryResult::is_null(int column_index) +{ + return true; +} + const char * QueryResult::get_value_from_result(int row, int col) { return nullptr; diff --git a/src/queryresult.h b/src/queryresult.h index 5f22d8d..f18e032 100644 --- a/src/queryresult.h +++ b/src/queryresult.h @@ -69,6 +69,7 @@ struct QueryResult */ virtual int get_column_index(const char * column_name); virtual int get_column_index(const wchar_t * column_name); + virtual bool is_null(int column_index); // may it should be changed to size_t? virtual const char * get_value_from_result(int row, int col);