Skip to content

Commit

Permalink
grammar: Simplify code
Browse files Browse the repository at this point in the history 
Simplify the code by storing the flag that indicates if the parsing was
successful in the parsed object itself instead of handing around pairs
of parsed objects and bools.
  • Loading branch information
@MKleusberg
MKleusberg committed Jan 20, 2017
1 parent 5455fd8 commit 6684fc2
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 71 deletions.
8 changes: 2 additions & 6 deletions src/EditTableDialog.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,8 @@ EditTableDialog::EditTableDialog(DBBrowserDB& db, const QString& tableName, bool
if(m_bNewTable == false)
{
// Existing table, so load and set the current layout
sqlb::TablePtr obj = pdb.getObjectByName(curTable).dynamicCast<sqlb::Table>();
QString sTablesql = obj->originalSql();
QPair<sqlb::ObjectPtr, bool> parse_result = sqlb::Table::parseSQL(sTablesql);
m_table = *(parse_result.first.dynamicCast<sqlb::Table>());
m_table.setTemporary(obj->isTemporary());
ui->labelEditWarning->setVisible(!parse_result.second);
m_table = *(pdb.getObjectByName(curTable).dynamicCast<sqlb::Table>());
ui->labelEditWarning->setVisible(!m_table.fullyParsed());

// Set without rowid and temporary checkboxex. No need to trigger any events here as we're only loading a table exactly as it is stored by SQLite, so no need
// for error checking etc.
Expand Down
2 changes: 1 addition & 1 deletion src/sqlitedb.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,7 @@ void DBBrowserDB::updateSchema( )
DBBrowserObject obj(val_name, val_sql, type, val_tblname);
if((type == sqlb::Object::ObjectTypes::Table || type == sqlb::Object::ObjectTypes::Index) && !val_sql.isEmpty())
{
obj.object = sqlb::Object::parseSQL(type, val_sql).first;
obj.object = sqlb::Object::parseSQL(type, val_sql);
if(val_temp == "1")
obj.object->setTemporary(true);
}
Expand Down
6 changes: 3 additions & 3 deletions src/sqlitetablemodel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ void SqliteTableModel::setTable(const QString& table, const QVector<QString>& di
bool allOk = false;
if(m_db.getObjectByName(table)->type() == sqlb::Object::ObjectTypes::Table)
{
sqlb::TablePtr t = sqlb::Table::parseSQL(m_db.getObjectByName(table)->originalSql()).first.dynamicCast<sqlb::Table>();
if(t->fields().size()) // parsing was OK
sqlb::TablePtr t = m_db.getObjectByName(table).dynamicCast<sqlb::Table>();
if(t && t->fields().size()) // parsing was OK
{
m_headers.push_back(t->rowidColumn());
m_headers.append(t->fieldNames());
Expand Down Expand Up @@ -432,7 +432,7 @@ QModelIndex SqliteTableModel::dittoRecord(int old_row)
int firstEditedColumn = 0;
int new_row = rowCount() - 1;

sqlb::TablePtr t = sqlb::Table::parseSQL(m_db.getObjectByName(m_sTable)->originalSql()).first.dynamicCast<sqlb::Table>();
sqlb::TablePtr t = m_db.getObjectByName(m_sTable).dynamicCast<sqlb::Table>();

sqlb::FieldVector pk = t->primaryKey();
for (int col = 0; col < t->fields().size(); ++col) {
Expand Down
54 changes: 24 additions & 30 deletions src/sqlitetypes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ QStringList fieldVectorToFieldNames(const FieldVector& vector)
return result;
}

QPair<ObjectPtr, bool> Object::parseSQL(Object::ObjectTypes type, const QString& sSQL)
ObjectPtr Object::parseSQL(Object::ObjectTypes type, const QString& sSQL)
{
// Parse SQL statement according to type
QPair<ObjectPtr, bool> result;
ObjectPtr result;
switch(type)
{
case Object::ObjectTypes::Table:
Expand All @@ -35,11 +35,11 @@ QPair<ObjectPtr, bool> Object::parseSQL(Object::ObjectTypes type, const QString&
result = Index::parseSQL(sSQL);
break;
default:
return QPair<ObjectPtr, bool>(ObjectPtr(nullptr), false);
return ObjectPtr(nullptr);
}

// Strore the original SQL statement and return the result
result.first->setOriginalSql(sSQL);
result->setOriginalSql(sSQL);
return result;
}

Expand Down Expand Up @@ -277,7 +277,7 @@ bool Table::hasAutoIncrement() const
return false;
}

QPair<ObjectPtr, bool> Table::parseSQL(const QString &sSQL)
ObjectPtr Table::parseSQL(const QString &sSQL)
{
std::stringstream s;
s << sSQL.toStdString();
Expand All @@ -294,11 +294,7 @@ QPair<ObjectPtr, bool> Table::parseSQL(const QString &sSQL)
parser.createtable();
CreateTableWalker ctw(parser.getAST());

// Note: this needs to be done in two separate lines because otherwise the optimiser might decide to
// fetch the value for the second part of the pair (the modify supported flag) first. If it does so it will
// always be set to true because the table() method hasn't run yet and it's only set to false in there.
sqlb::TablePtr tab = ctw.table();
return qMakePair(tab, ctw.modifysupported());
return ctw.table();
}
catch(antlr::ANTLRException& ex)
{
Expand All @@ -309,7 +305,7 @@ QPair<ObjectPtr, bool> Table::parseSQL(const QString &sSQL)
qCritical() << "Sqlite parse error: " << sSQL; //TODO
}

return qMakePair(TablePtr(new Table("")), false);
return TablePtr(new Table(""));
}

QString Table::sql() const
Expand Down Expand Up @@ -487,6 +483,7 @@ QString columnname(const antlr::RefAST& n)
TablePtr CreateTableWalker::table()
{
Table* tab = new Table("");
tab->setFullyParsed(true);

if( m_root ) //CREATE TABLE
{
Expand Down Expand Up @@ -521,7 +518,7 @@ TablePtr CreateTableWalker::table()
s = s->getNextSibling(); // USING
s = s->getNextSibling(); // module name
tab->setVirtualUsing(concatTextAST(s, true));
m_bModifySupported = false;
tab->setFullyParsed(false);

// TODO Maybe get the column list using the 'pragma table_info()' approach we're using for views

Expand Down Expand Up @@ -585,7 +582,7 @@ TablePtr CreateTableWalker::table()
|| tc->getType() == sqlite3TokenTypes::DESC))
{
// TODO save ASC / DESC information?
m_bModifySupported = false;
tab->setFullyParsed(false);
tc = tc->getNextSibling();
}

Expand Down Expand Up @@ -623,7 +620,7 @@ TablePtr CreateTableWalker::table()
|| tc->getType() == sqlite3TokenTypes::DESC))
{
// TODO save ASC / DESC information?
m_bModifySupported = false;
tab->setFullyParsed(false);
tc = tc->getNextSibling();
}

Expand Down Expand Up @@ -692,7 +689,7 @@ TablePtr CreateTableWalker::table()
break;
default:
{
m_bModifySupported = false;
tab->setFullyParsed(false);
}
break;
}
Expand Down Expand Up @@ -760,7 +757,7 @@ void CreateTableWalker::parsecolumn(Table* table, antlr::RefAST c)
if(con != antlr::nullAST && (con->getType() == sqlite3TokenTypes::ASC
|| con->getType() == sqlite3TokenTypes::DESC))
{
m_bModifySupported = false;
table->setFullyParsed(false);
con = con->getNextSibling(); //skip
}
if(con != antlr::nullAST && con->getType() == sqlite3TokenTypes::AUTOINCREMENT)
Expand All @@ -771,7 +768,7 @@ void CreateTableWalker::parsecolumn(Table* table, antlr::RefAST c)
{
// TODO Support constraint names here
if(!constraint_name.isEmpty())
m_bModifySupported = false;
table->setFullyParsed(false);

notnull = true;
}
Expand All @@ -785,7 +782,7 @@ void CreateTableWalker::parsecolumn(Table* table, antlr::RefAST c)
{
// TODO Support constraint names here
if(!constraint_name.isEmpty())
m_bModifySupported = false;
table->setFullyParsed(false);

con = con->getNextSibling(); //LPAREN
check = concatTextAST(con, true);
Expand All @@ -799,7 +796,7 @@ void CreateTableWalker::parsecolumn(Table* table, antlr::RefAST c)
{
// TODO Support constraint names here
if(!constraint_name.isEmpty())
m_bModifySupported = false;
table->setFullyParsed(false);

con = con->getNextSibling(); //SIGNEDNUMBER,STRING,LPAREN
defaultvalue = concatTextAST(con);
Expand All @@ -809,7 +806,7 @@ void CreateTableWalker::parsecolumn(Table* table, antlr::RefAST c)
{
// TODO Support constraint names here
if(!constraint_name.isEmpty())
m_bModifySupported = false;
table->setFullyParsed(false);

unique = true;
}
Expand Down Expand Up @@ -844,7 +841,7 @@ void CreateTableWalker::parsecolumn(Table* table, antlr::RefAST c)
break;
default:
{
m_bModifySupported = false;
table->setFullyParsed(false);
}
break;
}
Expand Down Expand Up @@ -944,7 +941,7 @@ QString Index::sql() const
return sql + ";";
}

QPair<ObjectPtr, bool> Index::parseSQL(const QString& sSQL)
ObjectPtr Index::parseSQL(const QString& sSQL)
{
std::stringstream s;
s << sSQL.toStdString();
Expand All @@ -961,11 +958,7 @@ QPair<ObjectPtr, bool> Index::parseSQL(const QString& sSQL)
parser.createindex();
CreateIndexWalker ctw(parser.getAST());

// Note: this needs to be done in two separate lines because otherwise the optimiser might decide to
// fetch the value for the second part of the pair (the modify supported flag) first. If it does so it will
// always be set to true because the table() method hasn't run yet and it's only set to false in there.
sqlb::IndexPtr index = ctw.index();
return qMakePair(index, ctw.modifysupported());
return ctw.index();
}
catch(antlr::ANTLRException& ex)
{
Expand All @@ -976,12 +969,13 @@ QPair<ObjectPtr, bool> Index::parseSQL(const QString& sSQL)
qCritical() << "Sqlite parse error: " << sSQL; //TODO
}

return qMakePair(IndexPtr(new Index("")), false);
return IndexPtr(new Index(""));
}

IndexPtr CreateIndexWalker::index()
{
Index* index = new Index("");
index->setFullyParsed(true);

if(m_root) // CREATE INDEX
{
Expand Down Expand Up @@ -1030,7 +1024,7 @@ IndexPtr CreateIndexWalker::index()
if(s->getType() != sqlite3TokenTypes::WHERE)
{
// It is something else
m_bModifySupported = false;
index->setFullyParsed(false);
} else {
s = s->getNextSibling(); // expr
index->setWhereExpr(concatTextAST(s, true));
Expand Down Expand Up @@ -1089,7 +1083,7 @@ void CreateIndexWalker::parsecolumn(Index* index, antlr::RefAST c)
break;
default:
// TODO Add support for COLLATE
m_bModifySupported = false;
index->setFullyParsed(false);
}

c = c->getNextSibling();
Expand Down
29 changes: 11 additions & 18 deletions src/sqlitetypes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class Object
Trigger
};

explicit Object(const QString& name): m_name(name), m_temporary(false) {}
explicit Object(const QString& name): m_name(name), m_temporary(false), m_fullyParsed(false) {}
virtual ~Object() {}

virtual ObjectTypes type() const = 0;
Expand All @@ -57,6 +57,9 @@ class Object

virtual QString baseTable() const { return QString(); }

void setFullyParsed(bool fully_parsed) { m_fullyParsed = fully_parsed; }
bool fullyParsed() const { return m_fullyParsed; }

virtual void clear() {}

/**
Expand All @@ -69,15 +72,15 @@ class Object
* @brief parseSQL Parses the CREATE statement in sSQL.
* @param sSQL The type of the object.
* @param sSQL The create statement.
* @return A pair. The first part is the parsed database object, the second a bool indicating if our
* parser fully understood the statement. The object may be empty if parsing failed.
* @return The parsed database object. The object may be empty if parsing failed.
*/
static QPair<ObjectPtr, bool> parseSQL(Object::ObjectTypes type, const QString& sSQL);
static ObjectPtr parseSQL(Object::ObjectTypes type, const QString& sSQL);

protected:
QString m_name;
QString m_originalSql;
bool m_temporary;
bool m_fullyParsed;
};

class Constraint
Expand Down Expand Up @@ -272,11 +275,9 @@ class Table : public Object
/**
* @brief parseSQL Parses the create Table statement in sSQL.
* @param sSQL The create table statement.
* @return A pair first is a table object, second is a bool indicating
* if our modify dialog supports the table.
* The table object may be a empty table if parsing failed.
* @return The table object. The table object may be empty if parsing failed.
*/
static QPair<ObjectPtr, bool> parseSQL(const QString& sSQL);
static ObjectPtr parseSQL(const QString& sSQL);
private:
QStringList fieldList() const;
bool hasAutoIncrement() const;
Expand All @@ -298,18 +299,15 @@ class CreateTableWalker
public:
explicit CreateTableWalker(antlr::RefAST r)
: m_root(r)
, m_bModifySupported(true)
{}

TablePtr table();
bool modifysupported() const { return m_bModifySupported; }

private:
void parsecolumn(Table* table, antlr::RefAST c);

private:
antlr::RefAST m_root;
bool m_bModifySupported;
};

QStringList fieldVectorToFieldNames(const sqlb::FieldVector& vector);
Expand Down Expand Up @@ -381,11 +379,9 @@ class Index : public Object
/**
* @brief parseSQL Parses the CREATE INDEX statement in sSQL.
* @param sSQL The create index statement.
* @return A pair. The first part is an index object, the second a bool indicating
* if our parser fully understood the statement.
* The index object may be if parsing failed.
* @return The index object. The index object may be empty if the parsing failed.
*/
static QPair<ObjectPtr, bool> parseSQL(const QString& sSQL);
static ObjectPtr parseSQL(const QString& sSQL);

private:
bool m_unique;
Expand All @@ -404,18 +400,15 @@ class CreateIndexWalker
public:
explicit CreateIndexWalker(antlr::RefAST r)
: m_root(r)
, m_bModifySupported(true)
{}

IndexPtr index();
bool modifysupported() const { return m_bModifySupported; }

private:
void parsecolumn(Index* index, antlr::RefAST c);

private:
antlr::RefAST m_root;
bool m_bModifySupported;
};

} //namespace sqlb
Expand Down
Loading

0 comments on commit 6684fc2

Please sign in to comment.