Show Data Module Review

This is the review page for the initial revision of the Show Data module.

Review process:

  • Reviewers add their comments as sections below using the
    header format. The section header should have your initials, a unique identifier, and a quick description (see below for examples)
  • Ahi's team responds by adding comments to the section
  • If the reviewer is satisfied, append "(Resolved)" to the section header
  • If the reviewer believes this needs more discussion, append "(Unresolved)" to the section header
  • All Unresolved comments will be discussed at the i-team on Tuesday June 10

Thanks everyone!


DVC-00 - Integration (Resolved)

Resolution is being handled as a mini-spec at DBDataViewerIntegration

OK, after looking at this code, this actually looks pretty doable.

It appears that the entry point into the module is through DataViewOutputPanel

    public void actionPerformed(ActionEvent ev) {
        try {
            String str = "";
            Document doc = DataObjectProvider.getProvider().getActiveDataObject().getDVEditorSupport().getDocument();
            str = doc.getText(0, doc.getLength());
            DatabaseConnection dbConn = DataViewSourceMultiViewElement.getSelectedConnection();
            DataViewOutputPanel  dataView = outputDataViewMap.get(str);
            if (dataView == null) {
                dataView = new SQLViewDataPanel(dbConn, str);
                outputDataViewMap.put(str, dataView);
        } catch (Exception ex) {

    public void showSplitPaneView(Component c) {
        // add to output.
        DataViewOutputWindowTopComponent topComp = DataViewOutputWindowTopComponent.findInstance();
        if (!topComp.isOpened()) {

It looks like a given output panel can display results for a single statement. So if there is a SQL Editor with multiple statements that generate results, then each one is going to result in a separate panel in the output window.

That seems reasonable to me.

The code that executes statements is SQLExecuteHelper in the db.core module, the class is
. In particular, in the execute() method, you'll see this code snippet:
            StatementInfo info = (StatementInfo);
            String sql = info.getSQL();

After this, it executes the SQL and then if the SQL is one that generates results, it passes the results on to the code that displays the results.

So instead of that, we could do something like (hand-waving here, but you get the idea)

            StatementInfo info = (StatementInfo);
            String sql = info.getSQL();

            DataViewOutputPanel  dataView = outputDataViewMap.get(sql);
            if (dataView == null) {
                dataView = new DataViewOutputPanel (dbConn, sql);
                outputDataViewMap.put(sql, dataView);

One detail is we should probably not create a new output panel for statements that don't generate results, but instead send that output directly to a single output window. Otherwise things will get ugly for large DDL/DML scripts. I think this is something probably better done in the Show Data module than at our level, since you're the one executing the SQL.

Another issue: we can't refer to DataViewOutputPanel directly, because the DataView module depends on Database Explorer, so the Database Explorer can't depend on DataView. So we need to create an SPI that allows you to register some kind of interface with Database Explorer that we can then invoke without a direct dependency.

More detailed comments follow, first on the user experience, and then on the implementation.

Ahi: All the above observations sounds good to me and Nithya will need some help in integrating this. I agree with your comments on none query statement execution. I have defined a spi and now DataView does not depend on Database Explorer, I hope this will make it easy to integrate.

Andrei: DataView can depend on the DB Explorer, although it probably doesn't need to.

Andrei: It seems it does need to depend on the DB Explorer in order to connect to the database if the connection is closed.

Ahi: The connection is provided by the caller, so DataView will not manage connection.

DVC-01 - Output as output window (Resolved)

Resolution is being finalized at DBDataViewerIntegration

I think it makes sense to have the output as an output window. I also really like it that each execution results in a new tab. This is what Wade was talking about on the dev alias. We should have him review this at some point

Andrei: Why? What are the disadvantages of the current solution in the SQL editor? I can think of advantages: the SQL and the results are close to each other (the current position of the data view is the worst possible, because you have to switch tabs), and there is a straightforward mapping of the editor window to the results window. Moreover, where do you propose to display the result of the script (which is currently displayed in the output window)?

Ahi: I have no issue with the current approach. The DataView can additionally provide an option to render the output in a tabbed output pane, currently it displays in the same frame as editor, but ideally it should be docked in the output area, that way user does not have to switch tab to see the output. For script execution, the SELECTs can be sent to DataView output window and other output can be either sent to generic netbeans output window.

DVC-02 - Inserting a new row (Unresolved)

It was surprising to me that when I chose "insert new row" it brought up a dialog. I was expecting it to just add a row to the results. This is how SQLYog works. Any reason we can't do this?

Ahi: This is doable, we were following db visualizer style, we thought we will also display the column data type as well and a check box for NULL value insertion while user leave the entry empty. If we decide to go with your suggestion that should not take much time, but my concern is where to add this new row? Say, if the page size is n when user attempt to insert, adding the new row in same page will make it n+1, where as now we pop-up insert dialog and then insert in the end. I think the a dialog should not be a big issue, we can use little more better layout, to make it look good.

David: We discussed this at the i-team, and had concluded that we could continue using the popup, with better layout. But before we go that route, I'd like to ask one more question: What about appending the new row to the bottom of the page, and after committing, that new row disappears and the results are refreshed?

DVC-03 - Explicitly applying changes (Resolved)

Resolution: stick with explicit commit

I notice I have to explicitly apply my changes before they are persisted. What are the reasons for doing it this way instead of applying each change as I go? That was what I was expecting...

Ahi: I think I like explicit commit instead of auto commit, say I have a composite key and some FKs in this case I would like like to apply my changes each time I change a column value.

Andrei: I agree. Although you could consider committing the row when the user selects another one. But that would require an option (keep the user in control).

DVC-04 - Tabbing after editing a column (Resolved)

Resolution: tracking this through Issue 136967

I double-click on a column, then edit it, then tab, and I go back out of edit mode. My expectation is to stay in edit mode until I move to another row.

Ahi: We are using JTable, may be we can customize this in future.

DVC-05 - Deleting a row failure (Resolved)

Resolution: I can't reproduce

I selected a row, chose the toolbar to delete the row, and I got "Delete command failed for Syntax error: "<EOF" at line 1, column 30."

Ahi: Delete does work for me; there might be some constraint violation. Try to insert a row and delete that row.

DVC-06 - Debugging (Resolved)

Resolution: Short-term, log all SQL using the logger API. If possible, provide an option to show the SQL that will be applied *before* you apply the change. Also, if the commit fails, the dialog should have a "show details" button that shows the SQL being executed. I think this still needs some work. I'll create an issue

Is there a simple way to show the SQL that is being executed when I do things like insert a row, edit a column, delete a row, etc.

Ahi: We can provide right click menu option and show insert/update delete sql for the selected row. I have kept a placeholder now, right click on the table area to see this. Also observe the column tool tip, this will also help while inserting and updating rows.

Andrei: If you aren't doing it already (you seem to, but I'm not sure you are doing it consistently), please make sure to log every statement you send to the database to a java.util.logging.Logger.

Ahi: We are logging it, but it spits out the prepared statement, user might want to see the sql that has values as well...

DVC-07 - Page size - nice! (Resolved)

Page size feature is great to have, thanks!

DVC-08 - Column headers (Resolved)

Is there a reason the header names are truncated? Shouldn't we expand each column to fit the size of the header text?

Ahi: Fixed.

DVC-09 - Database Metadata Model API (Resolved)

Resolution: We agree to collaborate on this, but will focus on the Show Data functionality for now.

Well, you guys seem to have implemented a basic metadata model API. We had that on our list to do as well. We should discuss whether this is something we want to use as the beginning for a more generic API. At first glance it looks cleanly designed. Andrei, perhaps you want to take a closer look?

Ahi: Good to hear that; I gave you a striped version of what we have and related table only. In the next step we would ideally parse the query and convert into db object model. So that the SELECT statement would require table and column object as well as Condition, GroupBy, OrderBy, Operator/functions etc. (this will also help in developing visual sql builder)

Andrei: unfortunately the model doesn't meet some of the constraints on DatabaseMetadataModel.

Ahi: May be we work together to make the model meet the constrains, may be we can write a separate review page for this?

DVC-10 - Internationalization (Resolved)

Just a heads up, I noticed that your error strings are not internationalized. This needs to be fixed prior to release.

Ahi: We had some dependencies on common SOA internal module, so we remove internationalized code for now. Nithya will fix this soon.

DVC-11 - Unit testing (Resolved)

Resolution Ahi's team commits to writing unit tets

No unit tests? Hm... That needs to get fixed.

Ahi: We have some junit for DB model that can be added here, and we will add few more ...

Andrei:: the DB model seems mostly used for column tooltips. Please make sure you add unit tests for the important stuff, like the generated queries.

Ahi: Agree we will add adequate test cases before release. I think DB Model is used in various other places other than tooltips.

DVC-12 - DataViewOutputPanel (Resolved)

Resolution: Ahi's team commits to clean this up. This includes separating SQL execution code from UI

Most of your code is nicely written and modular. This class, however, particularly the protected constructor, is different. It's a massive method that does a lot of things, many of which are not clear. However, this appears to be the key class where most of the work is done tying together the query being executed and the database metadata model that is used to map the query to the underlying database. We should probably walk through this class in particular to try and understand it and the logic behind it.

Ahi: I have made some changes, checkout the latest code.

DVC-13 - DataViewOutputPanel and exceptions (Resolved)

Resolution: Don't swallow exceptions

I am noticing that most exceptions are being 'swallowed' - you catch it, print a stack trace, and continue. For example,

        this.table = dbTable;
        if (dbTable instanceof DBTable) {
            try {
                String dbTypeStr = DBMetaDataFactory.getDBTypeFromURL(dbConn.getDatabaseURL());
                this.meta = new DBTableMetadata((DBTable) this.table, DBUtils.getSupportedDBType(dbTypeStr));
            } catch (Exception ex) {

I would think that everything is broken if, for example, the database type is not supported. What actually happens if the connection is for an unsupported database?

Ahi: Fixed. I think if we don't support a database then it sets the value to ANSI92, Yes some places exceptions are being 'swallowed';

DVC-15 - DataViewOutputPanel.createTable (Resolved)

This is not high priority, but I don't follow the logic of this method

    private DBObject createTable(String[[ | ]][] tableList, String tableName) {
        String[] currTable = null;
        DBObject dbTable = null;
        if (tableList != null) {
            for (int i = 0; i < tableList.length; i++) {
                currTable = tableList[I];
                dbTable = new DBTableImpl(currTable[[DBMetaDataFactory.NAME | DBMetaDataFactory.NAME]], currTable[[DBMetaDataFactory.SCHEMA | DBMetaDataFactory.SCHEMA]], currTable[DBMetaDataFactory.CATALOG]);
        return dbTable;

It appears to iterate through a list of tables, setting dbTable over and over again, and then returns the last one. Can you explain?

Ahi: Fixed.

DVC-16 DataViewOutputPanel - old code? (Resolved)

I'm noticing that this class also uses GridBagLayout and is not designed using Matisse. So this is pretty old code, right? But since you guys are maintaining this, I'll leave it to you to figure out how to deal with that :)

Ahi: Yes this is old code. May be we should use Matisse now !

AB-01 - javax.swing.RowSorter

This class was introduced in Java SE 6, but NetBeans must run on Java SE 5, so please do not use it.

Ahi: OK. Removed, lets make the basic part working, we may want to skip the matchbox feature for now.

AB-02 - Error handling

I opened the editor for the CUSTOMER table in the sample Java DB database, inserted a new row, only entered a value for the ID column. I got an error saying 'Insert command failed for For input string "" - Root cause: For input string: ""'.

SQL syntax errors are reported as exceptions, for example for "select foo bar baz" I got an AIOOBE.

Sometimes when an exception is thrown the Loading Data progress bar runs forever. You probably need to enclose calls to the progress API in try...finally.

Ahi: We will look into all these issues and fix before we checkin. May be we should just the current reporting style as NB SQL Editor

AB-03 - Connection handling

I get the following pretty often. Please do not close a java.sql.Connection obtained from the DB Explorer.

com.mysql.jdbc.exceptions.jdbc4.MySQLNonTransientConnectionException: No operations allowed after connection closed.
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
	at java.lang.reflect.Constructor.newInstance(
	at com.mysql.jdbc.Util.handleNewInstance(
	at com.mysql.jdbc.Util.getInstance(
	at com.mysql.jdbc.SQLError.createSQLException(
	at com.mysql.jdbc.SQLError.createSQLException(
	at com.mysql.jdbc.SQLError.createSQLException(
	at com.mysql.jdbc.ConnectionImpl.checkClosed(
	at com.mysql.jdbc.ConnectionImpl.getMetaData(
	at org.netbeans.modules.db.model.api.DBMetaDataFactory.getDBMetaData(
	at org.netbeans.modules.db.model.api.DBMetaDataFactory.connectDB(
	at org.netbeans.modules.db.dataview.output.DataViewOutputPanel.createDBTableFromQueryString(
	at org.netbeans.modules.db.dataview.output.DataViewOutputPanel.<init>(
	at org.netbeans.modules.db.dataview.test.ExecuteQuery.actionPerformed(
	at org.netbeans.modules.db.dataview.test.DataViewSourceMultiViewElement$2.actionPerformed(
	at javax.swing.AbstractButton.fireActionPerformed(
	at javax.swing.AbstractButton$Handler.actionPerformed(
	at javax.swing.DefaultButtonModel.fireActionPerformed(
	at javax.swing.DefaultButtonModel.setPressed(
	at javax.swing.plaf.basic.BasicButtonListener.mouseReleased(
	at java.awt.Component.processMouseEvent(
	at javax.swing.JComponent.processMouseEvent(
	at java.awt.Component.processEvent(
	at java.awt.Container.processEvent(
	at java.awt.Component.dispatchEventImpl(
	at java.awt.Container.dispatchEventImpl(
	at java.awt.Component.dispatchEvent(
	at java.awt.LightweightDispatcher.retargetMouseEvent(
	at java.awt.LightweightDispatcher.processMouseEvent(
	at java.awt.LightweightDispatcher.dispatchEvent(
	at java.awt.Container.dispatchEventImpl(
	at java.awt.Window.dispatchEventImpl(
	at java.awt.Component.dispatchEvent(
[Catch] at java.awt.EventQueue.dispatchEvent(
	at org.netbeans.core.TimableEventQueue.dispatchEvent(
	at java.awt.EventDispatchThread.pumpOneEventForFilters(
	at java.awt.EventDispatchThread.pumpEventsForFilter(
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(
	at java.awt.EventDispatchThread.pumpEvents(
	at java.awt.EventDispatchThread.pumpEvents(

Ahi: Fixed.

Andrei: Seemingly by doing your own connection management, and perhaps also because of the DB Explorer comments in DVC-00. See AB-16.

Ahi: No, I have responded AB-16

AB-04 - Connections duplicated in connection combo box.

You can use DatabaseExplorerUIs.connect() to get a combo box with connections.

Ahi: Fixed.

AB-05 - Query parsing not robust enough

For "select * from exceptions\n" (note the end of line character at the end) I get a RuntimeException saying "table not found".

Ahi: Fixed, Works the same way as NB SQL Editor

Andrei: Looks fixed.

Also, please do not use RuntimeException to report errors.

Ahi: Fixed.

Andrei: Looks fixed.

For "select component, count(id) from exceptions group by component" I get the following exception. This query works fine in the NetBeans SQL editor.

com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'count(id) from exceptions group by component' at line 1
        at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at sun.reflect.NativeConstructorAccessorImpl.newInstance(
        at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
        at java.lang.reflect.Constructor.newInstance(
        at com.mysql.jdbc.Util.handleNewInstance(
        at com.mysql.jdbc.Util.getInstance(
        at com.mysql.jdbc.SQLError.createSQLException(
        at com.mysql.jdbc.SQLError.createSQLException(
        at com.mysql.jdbc.MysqlIO.checkErrorPacket(
        at com.mysql.jdbc.MysqlIO.checkErrorPacket(
        at com.mysql.jdbc.MysqlIO.sendCommand(
        at com.mysql.jdbc.MysqlIO.sqlQueryDirect(
        at com.mysql.jdbc.ConnectionImpl.execSQL(
        at com.mysql.jdbc.ConnectionImpl.execSQL(
        at com.mysql.jdbc.StatementImpl.executeQuery(
        at org.netbeans.modules.db.dataview.output.DataViewWorkerThread.showDataForDBTable(
Caused: org.netbeans.modules.db.model.api.DBException: com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'count(id) from exceptions group by component' at line 1 - Root Cause: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'count(id) from exceptions group by component' at line 1
        at org.netbeans.modules.db.dataview.output.DataViewWorkerThread.showDataForDBTable(
        at org.netbeans.modules.db.dataview.output.DataViewWorkerThread.construct(
Caused: java.lang.RuntimeException
        at org.netbeans.modules.db.dataview.output.DataViewWorkerThread.construct(
        at org.netbeans.modules.db.model.util.SwingWorker$
[Catch] at

For "select discount_code from customer where discount_code = 'H'" against the Java DB sample I get an error about invalid SQL too.

Ahi: Fixed.

Andrei: Looks fixed.

AB-06 - Refresh button

When the number of rows is smaller than the default page size, the page size is set to the number of rows. Then pressing Refresh after adding another row externally doesn't seem to do anything, because the new row is displayed on the next page. Should rather leave the page size at its default value.

Ahi: Fixed.

Andrei: Looks fixed.

AB-07 - BLOB fields not supported

For a table like

create table test(
    id integer not null,
    text_value varchar(50),
    blob_value varchar(50) for bit data

I couldn't insert or delete rows because of an attempt to give the BLOB column a String value.

Ahi: Fixed.

AB-08 - NULL values

There doesn't seem to be a way to to insert NULL values, or to set an existing column's value to NULL for the edited row.

Ahi: Fixed.

Andrei: I see the fix, but I don't agree with it. I think what it needs is a "Make NULL" checkbox next to the text field for each nullable column. Otherwise, what happens when I leave a non-nullable varchar field empty?

Ahi: Agree, good suggestion.

AB-09 - Performance

For a simple table like CUSTOMER in the sample Java DB database, the execution of a simple query seems more sluggish in the data view window than the NetBeans SQL editor. I think you are retrieving the database metadata (the list of tables, at least, in DBMetaDataFactory.getTables()) for each statement execution.

Ahi: DBMetaDataFactory.getTables() brings metadata for the table in subject, it always return one table; I don't think this is the issue. We are fundamentally executing the query twice one for the result set and one for the count. We will try to improve performance by introducing database specific optimization.

Andrei: NativeColumnOrderComparator should not be a singleton. It is cheaper to just create it when needed.

__Andrei: There is a Thread.sleep(5000) statement in DBExplorerUtil, please remove it and instead just make sure you call showConnectionDialog in a synchronous event thread action (SwingUtilities.invokeAndWait).

Ahi: OK,Fixed the above issues, but will look into various performance concern.

AB-10 - Result set handling not enclosed by try...finally.

Like in DataViewWorkerThread.showDataForDBTable().

Ahi: Fixed

Andrei: OK, fixed.

AB-11 - Precision for string columns

The column tooltip shows "precision" for string columns, but that is meaningless. It should be "length". Precision should only be used for numeric types.

Ahi: Fixed.

Andrei: OK, fixed.

AB-12 - Threading

The threading model of the data view panel is too asynchronous. Multiple actions can be executed in multiple threads at the same time. Instead, all database work should take place in a single, dedicated thread. Moreover, starting threads instead of using a thread pool is a NetBeans antipattern.

Some database calls are executed in the AWT event dispatching thread, like in the constructor of DataViewOutputPanel, or ResultSetTablePanel.executeDeleteRow() or DataViewOutputPanel.insertActionPerformed().

The db model doesn't seem to be thread safe.

Ahi: Agree. Lets fix this once we have a base.

AB-13 - Internationalization

Many NOI18N markers missing.

Ahi: We will fix these before checking in

AB-14 - Too many assumptions about user's SQL

The data view must reasonably work with any SELECT statement. But DataViewWorkerThread.getCountSQLQuery() makes too strong assumptions about the statement.

Ahi: Fixed

Andrei: OK, fixed.

AB-15 - Paging

The whole idea of paging is that you can retrieve data quickly. For instance, when you have a large table, you don't have to wait for all the table to be retrieved in order to see the first 10 rows. But that's not how the paging in the data view window works. It always executes the user's query, and only displays the relevant part of the result. So for MySQL the data view is almost as slow as the NetBeans SQL editor. Perhaps server-side cursors can help, not sure.

Ahi: I don't think it waits for the whole table to be retrieved before it shows data for the first 10 rows, to optimize further we are now setting resultset fetch size to give an hint to driver how much need to be fetched when we need more rows. Paging will have some speed advantage, but I think unless we customize for each database to limit the rows (oracle has rownum and some database support limit...offset syntax), and we also need to optimize the total count query.

Andrei: how do you set the fetch size? There doesn't seem to be any call to setFetchSize() in the zip from June 10 4pm CET.

Ahi: you have a wrong version may be...

Andrei:: right. However, you still need to fetch all rows up to startFrom, so that when startFrom is large you will still retrieve a lot of rows. Worse, when you move back, you will retrieve all those rows again. By the way, have you investigated scrollable result sets?

Ahi: scrollable resultset has too many complication and they behave differently for each driver and each database. The best choice is to figure out how to hint the database to roll up to startFrom before returning the resultset and the limit the fetch size to page size. ANSI SQL has spec for limit...offset, but not supported by all database, but there are some version of it like oracle support rownum for older version and limit...offset for the newer versions. My idea is to customize based on the database in use and a generic fall back approach.

AB-16 - Connection management

Connections to the database are made directly through java.sql classes instead of the DB Explorer. This is not right, please use the DB Explorer. There is no apparent reason not to depend on it.

Ahi: Not True. The connection Provider SPI provides the connection.

Andrei: OK. Old version.

AB-17 - not buildable on Java SE 5

Because of calls to JDBC4 methods of PreparedStatement in DBUtils.

Ahi: Fixed

AB-18 - handling of autogenerated columns

I could not insert a row in a Derby table with an identity column.

Ahi: Good catch, I will also look into generated columns or logical columns

Action Items

  • Integration
  • Inserting a new row -- better UI or Inline option
  • Provide API to retun JPanel with horizontal toolbar only
  • Error handling -- Instead of error dialog use output window, similar to NB SQL Editor
  • Performance -- Need to limit the row selection by the page and optimize the count(*) operation. Need to remove any database calls from the AWT event dispatching thread.
  • Threading -- all database work should take place in a single and use thread pool
  • Use RequestProcessor instead of SwingWorker
  • Database Metadata Model API - Make it thread safe
  • Debugging -- Provide a means to see the actual SQL for all edit operations.
  • Internationalization & missing NOI18N markers
  • Unit testing
  • Connection management
  • Generated Columns (the textbox should be disabled), Make Null Check box for each nullable column
  • DataViewOutputPanel Cleanup including separating SQL execution code from UI
  • Indication that the column has been modified and data validation based on type
Not logged in. Log in, Register

By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo